-
Notifications
You must be signed in to change notification settings - Fork 56
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
ENH: street_alignment and get_nearest_street #566
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #566 +/- ##
=======================================
+ Coverage 97.4% 97.7% +0.3%
=======================================
Files 26 32 +6
Lines 4328 4985 +657
=======================================
+ Hits 4214 4869 +655
- Misses 114 116 +2
|
|
||
|
||
def get_nearest_street( | ||
buildings: gpd.GeoSeries | gpd.GeoDataFrame, |
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.
Is there is specific reasoning to using both pd
and Series
above, and using gpd.GeoSeries
here? Should we also do from gpd import GeoSeries
to conform? Or vice-versa?
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.
No particular reason. My thought was that people will be familiar with Series but may not be with GeoSeries, so I wanted to indicate where it comes from. Do you have any preference here?
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.
My personal preference is "always explicit" so if I'd had my way it would be non-aliased & full usage: e.g.:
import pandas
...
pandas.Series
However, I know that my preference is not norm. If I had to choose one of the two methods already being used here, I'd probably go with non-aliased classes (e.g. from gpd import GeoSeries
) for the type hinting.
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.
I also know that you like linting rules, so have one for you - https://docs.astral.sh/ruff/rules/unconventional-import-alias/
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
I'll do the typing consistency as a follow-up across the codebase. |
I love how streamlined these things are now compared to the original :).