-
Notifications
You must be signed in to change notification settings - Fork 28
Use srtree from shapely instead of rtree package #214
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
Conversation
|
And this drops python 3.9 support and adds python 3.13 support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the rtree package dependency with Shapely's built-in STRtree to enable proper pickling/unpickling of matcher objects. It also adds pixi build tool support and updates Python version support (dropping 3.9, adding 3.13).
Key changes:
- Switched from
rtree.index.Indextoshapely.strtree.STRtreefor spatial indexing - Removed
rtreedependency from pyproject.toml - Added version constraints to all dependencies
- Configured pixi as an alternative build tool
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Removed rtree dependency, added version constraints, added pixi configuration, updated Python version support |
| mappymatch/maps/nx/nx_map.py | Replaced rtree with STRtree, updated nearest_road implementation |
| mappymatch/maps/igraph/igraph_map.py | Replaced rtree with STRtree, updated _nearest_edge_index implementation |
| mappymatch/matchers/lcss/lcss.py | Removed unused match_trace_batch method that had rtree serialization issues |
| .github/workflows/*.yml | Updated Python version matrix to 3.10-3.13 |
| environment*.yml | Deleted conda environment files (replaced by pixi) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This also introduces an optional parameter "additional_metadata_keys" into the |
|
@jhoshiko - this should be ready for your review! |
jhoshiko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, All of the changes looks good to me. I pulled the branch and ran all of the unit tests which are all passing and also put together a quick notebook to test the branch which also went flawlessly (all testing was python=3.13).
I also noticed your note about the memory usage while building the tree and I will also keep an eye out for memory issues in the future, but I think we can merge this in! Thanks for putting this together.
Updates the internal RTree to be the
SRTreefrom shapely rather than using thertreepackage. This allows us to pickle and unpickle the matcher without corrupting the index.Also adds support for the pixi build tool.