Add inject_points for projecting external points onto the network#305
Add inject_points for projecting external points onto the network#305jg-codes wants to merge 7 commits into
Conversation
martinfleis
left a comment
There was a problem hiding this comment.
A first pass without checking the tests yet.
Out of curiosity, what happens on your real use case when you use directly split with eps equal to what you use as snap_radius here? I have a feeling that you might actually get the same outcome as with inject_points.
| if streets.crs is None or points.crs is None: | ||
| raise ValueError("Both streets and points must have a CRS set.") | ||
| if points.crs != streets.crs: | ||
| points = points.to_crs(streets.crs) |
There was a problem hiding this comment.
I would do this here as we do in neatify - check and if it does not match raise. Let users fix it, rather than implicitly re-project.
| input_idx, line_idx = streets.sindex.nearest(point_geoms, return_all=False) | ||
| distances = shapely.distance(line_geoms[line_idx], point_geoms[input_idx]) |
There was a problem hiding this comment.
sindex.nearest can directly return distances - see docs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #305 +/- ##
=======================================
- Coverage 99.1% 98.8% -0.2%
=======================================
Files 7 7
Lines 1277 1299 +22
=======================================
+ Hits 1265 1284 +19
- Misses 12 15 +3
🚀 New features to boost your workflow:
|
- Raise on CRS mismatch instead of implicitly reprojecting, matching the behaviour of `neatify` and letting users fix mismatched inputs explicitly (review: check-and-raise rather than silent reprojection). - Replace the separate `shapely.distance` pass with `sindex.nearest(..., return_distance=True)`, which returns the point-to-line distances directly (one geometry pass instead of two). - Run ruff-format on the CRS test (fixes the failing pre-commit.ci check). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ions Clarify that inject_points projects to the foot of the perpendicular and adds a node *on* the existing geometry (no kink), unlike split() which snaps the line to the point; the two coincide only when the point already lies on the line. Narrow the stated scope to near-line features and document the limitations raised in review: point attributes are not carried over, the projection clamps to endpoints (which can be a no-op), and a genuinely off-network point needs a connector edge rather than inject_points. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Ah, great! Thanks a lot. We should make sure this comes clear through the docs, both docstrings of both functions and in the user guide notebook. The fact that |
…docstrings Per review: make the distinction explicit in both functions' docstrings. `split` now states that snapping forces the line through an off-line point (a kink) and points to `inject_points`; `inject_points` notes it keeps the original linework, just subdivided. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Show on a small example that split snaps the line through an off-line point (a kink) while inject_points projects the point onto the line and subdivides it, keeping the original geometry. Addresses the review request to make the distinction clear in the user guide notebook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
I've added the distinction to the docs. Let me know if anything else comes up your mind. |
| """Split lines on new nodes. | ||
| """Split lines at the given points, snapping each line through the point. | ||
|
|
||
| Each point is snapped onto the nearest line within ``eps`` (via |
There was a problem hiding this comment.
Line is snapped onto point, not point onto line, no?
| Each input point within ``snap_radius`` of the network is projected onto | ||
| its *nearest* LineString in ``streets`` -- at the foot of the | ||
| perpendicular, via linear referencing -- and that line is split at the | ||
| projected location. The new node therefore lands exactly *on* the original |
There was a problem hiding this comment.
Not exactly as we're snapping coordinates into a floating point precision grid but almost exactly. I would avoid using the word exactly here as the original and new lines will not overlap 100%.
|
|
||
| # Nearest line per input point, with distance (sindex-accelerated) | ||
| (input_idx, line_idx), distances = streets.sindex.nearest( | ||
| point_geoms, return_all=False, return_distance=True |
There was a problem hiding this comment.
If the snap_radius is set, we could pass it here as max_distance to speed this up.
| Point features to project onto ``streets``. Must share the same CRS as | ||
| ``streets`` (a mismatch raises rather than reprojecting implicitly). | ||
| Only the geometries are used; point attributes are not carried over. | ||
| snap_radius : float | None = None |
There was a problem hiding this comment.
Not sure if snap_radius is the right name as we're technically not snapping. Maybe keep just radius?


Summary
This PR adds
inject_points, a focused helper for projecting external Point features onto the nearest network LineString and splitting at the projected location.It follows the scoped discussion in #304: this PR includes only
inject_points, its public export, and the API reference entry. The separate node-attribute attachment helper is intentionally left out for a future discussion.What Changed
neatnet.inject_pointssnap_radiusfiltering, CRS reprojection, empty inputs, and multiple points on one edgeinject_pointsto the API referenceValidation
git diff --check upstream/main...HEADpasseslibpysalin this local environment; GitHub Actions should run the project environmentAI assistance disclosure: Prepared with AI assistance; final scope, structure, and review decisions were made by JG.