Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRAFT: try to remove code duplication for different datatypes using macro #8

Closed
wants to merge 1 commit into from

Conversation

evgenii-kuznetcov
Copy link
Contributor

Didn't want to open macro pandora box but this makes code much shorter. IDE support in vscode still works.
If we like the approach, I can add missing datatypes

@thobai
Copy link
Collaborator

thobai commented Nov 21, 2024

It does make the code shorter, but actually, I find the tdigest_cast version more elegant. We have to cast anyways, so we can also do that beforehand. There's no performance impact from that. What do you think?

@evgenii-kuznetcov
Copy link
Contributor Author

It does make the code shorter, but actually, I find the tdigest_cast version more elegant. We have to cast anyways, so we can also do that beforehand. There's no performance impact from that. What do you think?

Agree, tdigest_cast should be better. Didn't notice this function and it was not used

@thobai
Copy link
Collaborator

thobai commented Nov 22, 2024

ok, then we should make that one the default and actually remove the other one. But for that we also have to bring in the changes that you already did for the current function. Will you take this over or shall I do it sometime?

@evgenii-kuznetcov
Copy link
Contributor Author

See #13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants