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

Add an order &dedupe option #1568

Closed

Conversation

krader1961
Copy link
Contributor

Related #1453

@SolitudeSF
Copy link

why is that an option instead of separate command?

@krader1961
Copy link
Contributor Author

why is that an option instead of separate command?

For performance. It's more efficient to do the dedupe when ordering the values than it is to pipe them into a separate command. I'm also working on a separate command. See issue #1453.

@krader1961
Copy link
Contributor Author

@SolitudeSF, I just created P.R. #1569 that implements a dedupe command. So now we can efficiently sort lists with duplicate elimination and dedupe arbitrary inputs.

@xiaq
Copy link
Member

xiaq commented Aug 28, 2022

Sorry, but no.

Optimizing order | dedupe is indeed desirable, eventually. But it can and should be done transparently - the compiler can recognizing the pattern order | dedupe in a pipeline and use a more efficient implementation under the hood.

@xiaq xiaq closed this Aug 28, 2022
@krader1961 krader1961 deleted the issue-1453-order-compact-option branch August 29, 2022 04:19
@krader1961
Copy link
Contributor Author

Optimizing order | dedupe is indeed desirable, eventually. But it can and should be done transparently - the compiler can recognizing the pattern order | dedupe in a pipeline and use a more efficient implementation under the hood.

Yes, but I suspect I'll be dead before that happens. 😄

It's not clear to me why an obviously trivial optimization that works today, and the immediate future, isn't acceptable. Even if the optimization you envision is implemented in the distant future I don't see any reason to not implement the much simpler optimization today since it is trivial.

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.

None yet

3 participants