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

[WIP] Range queries #167

Merged
merged 13 commits into from
Aug 23, 2022
Merged

[WIP] Range queries #167

merged 13 commits into from
Aug 23, 2022

Conversation

ljwolf
Copy link
Contributor

@ljwolf ljwolf commented Jul 9, 2021

Hey folks, really looking forward to the functionality in the pandana/range-queries branch being merged. So... I've rebased it onto the development branch, added docstrings and tests, and finalised the return value to follow the pattern from nearest_pois().

The todos in cyaccess.pyx were well placed: I found that if you precompute to a given distance, the function would return all pairs within the precomputed distance, regardless of the radius. I tried to fix this in 63f01ff, but that resulted in bad destination ids & weights (my c++ knowledge only comes from troubleshooting Stan models 🤓, so I'm no expert). So, now, the radius filter happens in a pandas query after the results are returned. This is wasteful, so I'd be grateful to learn how to implement this directly within src/accessibility.cpp.

@knaaptime
Copy link

this would be killer in the next release! let me know if there's anything i can do to help facilitate :)

@knaaptime
Copy link

hey folks, just wanted to check in and see if you're up for merging this and cutting a new release. We'd love to use this new feature :). Happy to help in any way I can

@smmaurer
Copy link
Member

This is awesome, thanks @ljwolf!! Read through the code and ran the tests on my machine and it looks great. I'm going to merge this and release it along with a couple of the other outstanding PRs.

Sorry it's languished unmerged for so long. We've had limited resourcing but are trying to get more organized.

Copy link
Member

@smmaurer smmaurer left a comment

Choose a reason for hiding this comment

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

🎉

@smmaurer smmaurer merged commit b5b62f4 into UDST:dev Aug 23, 2022
@knaaptime
Copy link

sorry it took me so long to circle back, but its sweet to see this merged @smmaurer. Any chance to cut a new release with the latest? I'd be happy to help however I can!

With these enhancements we can get pandana-backed network spatial weights in libpysal 😎

@smmaurer smmaurer mentioned this pull request Jul 25, 2023
4 tasks
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