Conversation
Also trying to simplify code and create fewer copies of things
…te_route_at_stop() to operate in place
…s; inherit this classification from their children
Replace per-row .apply() with GeoSeries/Series numpy operations (addresses review comments on get_link_bearing_degrees and bearing_to_cardinal_direction). - get_link_bearing_degrees -> get_link_bearings_degrees(geometry: GeoSeries): extracts endpoints with shapely vectorized get_point/get_x/get_y and computes forward azimuths in one pyproj Geod.inv call; NaN where geometry is missing/empty/degenerate. - bearing_to_cardinal_direction -> bearings_to_cardinal_directions(bearings: Series): numpy sector floor-divide against _CARDINAL_LABELS/_INTERCARDINAL_LABELS; None where NaN. - add_direction_to_links now calls the vectorized helpers instead of two nested applies.
Replace the per-node modal-graph iteration with a vectorized groupby over modal links (mode_query): out-degree via size() and node fitness via max() of the incident links' *_centroid_fit. Import all node attributes through a single inner merge on nodes_df instead of a hardcoded column list, dropping the street_count dependency (which does not exist on every network) and the redundant fillna/astype cleanup. Addresses review comments L211, L236, L249.
Add tests covering add_centroid_nodes, add_centroid_connectors, and calculate_angle_from_centroid against the small example network. The connectors test deliberately omits street_count on the nodes so it guards the mode_query/groupby node evaluation, and asserts per-zone connector counts (not specific node choices) so it remains valid across future selection-loop refactors.
- Vectorize centroid distance and angle-bearing calculations in centroid connector selection. - Replace apply-heavy per-zone connector picking with incremental min-angle separation updates. - Refactor ML_geometry normalization into a dedicated helper to reduce full-column coercion work. - Harden parquet writes by adding a fallback that JSON-encodes nested object columns when Arrow conversion fails. - Add regression test coverage for parquet nested-object fallback behavior.
Collaborator
|
I see you are hard at work on this - LMK when you are ready for another review :-) |
…id; add projected-CRS guard
…ion, replace apply with list comprehensions
…ion, replace apply with list comprehensions
…ate docs; add ZonesTable TODO for zones_df on ModelRoadwayNetwork
…_nodes, _select_nodes_by_sector, _build_connector_links_df
Member
Author
I think I've addressed your comments and it's ready for another look! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds end-to-end support for creating centroids and centroid connectors in a model network, including the core APIs, geometry/link generation logic, and tests/documentation updates needed to use and validate the workflow.
What this PR does
This PR introduces functionality to build model-zone access structures by generating:
Key changes
Why this change is needed
Many demand and assignment workflows require zone-level connectors to represent how trips enter/exit the physical network. This PR provides a native, repeatable way to generate those structures directly in network_wrangler, reducing manual preprocessing and ensuring consistent connector generation behavior.
Implementation notes
Centroid generation and connector generation are implemented as explicit network-building operations so they can be run as part of a reproducible wrangling pipeline.
Connector creation is based on proximity/eligibility rules to attach centroids to nearby nodes.
The new functionality is intended to be configurable for model-specific connector policies.
Testing
This PR includes tests for:
Existing workflows are unaffected unless the new centroid/connector functionality is explicitly invoked. No breaking changes are intended for users not using these new APIs.
Limitations / follow-ups
Connector selection policies may still need tuning for specific model conventions (e.g., max connectors per zone, facility/type filtering, directional behavior).
Future enhancements can expand configurability and performance for very large zone sets.