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

droplevels!(): more efficient implementation #359

Merged
merged 5 commits into from
Aug 17, 2021

Conversation

alyst
Copy link
Contributor

@alyst alyst commented Jun 24, 2021

Replace the current one-line implementation of droplevels!() as behind the scenes it requires sorting and arrays intersection.
The new implementation scans the array only once, and doesn't rebuild the array when all levels are observed.

Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! If you want to make this even faster, you could skip some redundant checks that are done in levels! (uniqueness and addition of new levels) but that could be avoided by using an unsafe internal function.

src/array.jl Outdated Show resolved Hide resolved
src/array.jl Outdated Show resolved Hide resolved
src/array.jl Outdated Show resolved Hide resolved
@alyst
Copy link
Contributor Author

alyst commented Jul 27, 2021

do you think it is ready to be merged?

@bkamins
Copy link
Member

bkamins commented Jul 27, 2021

We might need to wait a bit as @nalimilan is OOO AFAICT.

@alyst
Copy link
Contributor Author

alyst commented Aug 13, 2021

gentle bump

@nalimilan
Copy link
Member

Feel free to merge PRs that I have approved (three times!). :-)

@nalimilan nalimilan merged commit 6803c3c into JuliaData:master Aug 17, 2021
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