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

[stdlib][Draft] Introduce heapq module #2999

Closed
wants to merge 1 commit into from

Conversation

bgreni
Copy link
Contributor

@bgreni bgreni commented Jun 9, 2024

Going down the list of "stuff we need because Python has it", we have the heapq module.

There exists a pure python implementation in the Python standard library, so for now we can follow it basically exactly, then write some benchmarks and tweak if from there if possible?

@bgreni bgreni force-pushed the add-heapq branch 2 times, most recently from d860e53 to 8a856a2 Compare June 9, 2024 16:53
@gabrieldemarmiesse
Copy link
Contributor

gabrieldemarmiesse commented Jun 9, 2024

Maybe we can wait a bit before introducing new data structures? We are constantly in the middle of rewrites (new hash, new type of copy, new to_format method instead of __str__). Let's wait until the dust is settled down and we feel good about our existing structs before adding new collections that will require those rewrites and maybe new rewrites that we have not anticipated yet.

I'm all for good python compatibility, but with heapq we would be biting more than we can chew at the moment.

@bgreni
Copy link
Contributor Author

bgreni commented Jun 9, 2024

Maybe we can wait a bit before introducing new data structures?

That's fair, but in this case the heapq module doesn't actually define any new collections, but rather a bunch of free functions to treat a List as a heap and only uses the most basic methods on List so I feel it should be largely unaffected by any of these rewrites?

@gabrieldemarmiesse
Copy link
Contributor

I'll let the maintainers chim in but in my opinion, we do not have enough bandwidth at the moment. Furthermore some rewrites may affect free functions (like references being constantly changing).

Signed-off-by: Brian Grenier <[email protected]>
@guidorice
Copy link

guidorice commented Sep 7, 2024

"stuff we need because Python has it"

heapq would be a nice enhancement because it's useful for AI related stuff, like Hierarchical Navigable Small World (vector similarity search). Also noting that python has a C implementation, so benchmarking against the mojo impl would be interesting- and important:
https://github.com/python/cpython/blob/033510e11dff742d9626b9fd895925ac77f566f1/Modules/_heapqmodule.c
https://github.com/python/cpython/blob/033510e11dff742d9626b9fd895925ac77f566f1/Lib/heapq.py#L581

@JoeLoser
Copy link
Collaborator

Any interest in rebasing this branch since the stdlib has come quite a ways since this was first introduced? We're happy to review once that is done.

@JoeLoser JoeLoser added the waiting for response Needs action/response from contributor before a PR can proceed label Nov 15, 2024
@bgreni
Copy link
Contributor Author

bgreni commented Nov 15, 2024

@JoeLoser I'm curious given the new attitude around loosening our commitment to being a "superset" if we might rather refactor this into it's own distinct data structure instead of a collection of functions operating on lists like in Python?

@JoeLoser
Copy link
Collaborator

@JoeLoser I'm curious given the new attitude around loosening our commitment to being a "superset" if we might rather refactor this into it's own distinct data structure instead of a collection of functions operating on lists like in Python?

I'm glad that you asked! Personally, yes, I'd go that direction. Do you have any interest in writing up a small API design sketch proposal and we can discuss and go that route before going too far? Feel free to consider inspiration from https://doc.rust-lang.org/std/collections/struct.BinaryHeap.html and friends.

@bgreni
Copy link
Contributor Author

bgreni commented Nov 15, 2024

I'm glad that you asked! Personally, yes, I'd go that direction. Do you have any interest in writing up a small API design sketch proposal and we can discuss and go that route before going too far? Feel free to consider inspiration from https://doc.rust-lang.org/std/collections/struct.BinaryHeap.html and friends.

Great I'd love to!

@bgreni
Copy link
Contributor Author

bgreni commented Nov 28, 2024

Closing this now that the proposal for a dedicated BinaryHeap struct is in progress #3776

@bgreni bgreni closed this Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for response Needs action/response from contributor before a PR can proceed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants