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

Superseed filter_by_sf() with filter_by_spatial_extent() and create crop_gtfs() #76

Open
dhersz opened this issue Nov 30, 2023 · 1 comment

Comments

@dhersz
Copy link
Member

dhersz commented Nov 30, 2023

filter_by_trip_geometry() would work exactly like filter_by_sf().

filter_by_stop_location() would, on the other hand, filter only based on stop locations. Given a bbox/polygon, the function would keep/drop the stops located inside the polygon and the entries related to them.

Both of them would receive a sf object, which would make the name filter_by_sf() a bit ambiguous (arguably it doesn't communicate the function behavior very well even considering the current behavior alone, as shown by the several issues that have been opened addressing this function). So I think the best way forward is to soft deprecate filter_by_sf() in the release, warning that filter_by_trip_geometry() should be used instead, and to hard deprecate it in version 2.0.0.

@dhersz
Copy link
Member Author

dhersz commented Dec 1, 2023

Ok, maybe the names are not the best.

Usually in filter_by_* functions the * is exactly what you have to supply to the function. E.g. filter_by_trip_id() receives trip_id and subsets the content based on that. In the case of filter_by_trip_geometry() and filter_by_stop_location() the user wouldn't pass the trip geometries and the stop locations. Rather, they would pass a spatial object and the filter behavior would change based on the function.

So, perhaps, to make the function behavior clearer we could rename filter_by_sf() to filter_by_spatial_extent() and what would be filter_by_stop_location() to crop_gtfs(). Feels better.

@dhersz dhersz changed the title Superseed filter_by_sf() with filter_by_trip_geometry() and create filter_by_stop_location() Superseed filter_by_sf() with filter_by_spatial_extent() and create crop_by_spatial_extent() Dec 1, 2023
@dhersz dhersz changed the title Superseed filter_by_sf() with filter_by_spatial_extent() and create crop_by_spatial_extent() Superseed filter_by_sf() with filter_by_spatial_extent() and create crop_gtfs() Dec 1, 2023
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

No branches or pull requests

1 participant