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.

Sign up for GitHub

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

docstring & typehints in simplify.network_simplify(), etc. #107

Merged
merged 4 commits into from
Nov 26, 2024
Merged

docstring & typehints in simplify.network_simplify(), etc. #107

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
25837b1
docstring from simplify.network_simplify() + others
jGaboardi Nov 24, 2024
122a551
return type hint in simplify_network()
jGaboardi Nov 24, 2024
b7f528f
forgot consolidation_tolerance keyword explanation
jGaboardi Nov 24, 2024
b11822e
martin comments & clarifications
jGaboardi Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions sgeop/artifacts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@

def get_artifacts(
roads: gpd.GeoDataFrame,
threshold: None | float = None,
threshold_fallback: None | float = None,
area_threshold_blocks: float = 1e5,
isoareal_threshold_blocks: float = 0.5,
area_threshold_circles: float = 5e4,
isoareal_threshold_circles_enclosed: float = 0.75,
isoperimetric_threshold_circles_touching: float = 0.9,
threshold: None | float | int = None,
threshold_fallback: None | float | int = None,
area_threshold_blocks: float | int = 1e5,
isoareal_threshold_blocks: float | int = 0.5,
area_threshold_circles: float | int = 5e4,
isoareal_threshold_circles_enclosed: float | int = 0.75,
isoperimetric_threshold_circles_touching: float | int = 0.9,
exclusion_mask: None | gpd.GeoSeries = None,
predicate: str = "intersects",
) -> tuple[gpd.GeoDataFrame, float]:
Expand All @@ -40,24 +40,24 @@ def get_artifacts(
----------
roads : geopandas.GeoDataFrame
Input roads that have been preprocessed.
threshold : None | float = None
threshold : None | float | int = None
First option threshold used to determine face artifacts. See the
``artifact_threshold`` keyword argument in ``simplify.simplify_network()``.
threshold_fallback : None | float = None
threshold_fallback : None | float | int = None
Second option threshold used to determine face artifacts. See the
``artifact_threshold_fallback`` keyword argument in
``simplify.simplify_network()``.
area_threshold_blocks : float = 1e5
area_threshold_blocks : float | int = 1e5
Areal theshold for block detection.
isoareal_threshold_blocks : float = 0.5
isoareal_threshold_blocks : float | int = 0.5
Isoareal theshold for block detection.
See ``esda.shape.isoareal_quotient``.
area_threshold_circles : float = 5e4
area_threshold_circles : float | int = 5e4
Areal theshold for circle detection.
isoareal_threshold_circles_enclosed : float = 0.75
isoareal_threshold_circles_enclosed : float | int = 0.75
Isoareal theshold for enclosed circle detection.
See ``esda.shape.isoareal_quotient``.
isoperimetric_threshold_circles_touching : float = 0.9
isoperimetric_threshold_circles_touching : float | int = 0.9
Isoperimetric theshold for enclosed circle touching.
See ``esda.shape.isoperimetric_quotient``.
exclusion_mask : None | gpd.GeoSeries = None
Expand Down
4 changes: 2 additions & 2 deletions sgeop/continuity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ def get_stroke_info(

Parameters
----------
artifacts : GeoSeries | GeoDataFrame
artifacts : geopandas.GeoSeries | geopandas.GeoDataFrame
Polygons representing the artifacts.
roads : GeoSeries | GeoDataFrame
roads : geopandas.GeoSeries | geopandas.GeoDataFrame
LineStrings representing the road network.

Returns
Expand Down
14 changes: 7 additions & 7 deletions sgeop/geometry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ def voronoi_skeleton(
lines: list | np.ndarray | gpd.GeoSeries,
poly: None | shapely.Polygon = None,
snap_to: None | gpd.GeoSeries = None,
max_segment_length: int = 1,
max_segment_length: float | int = 1,
buffer: None | float | int = None,
secondary_snap_to: None | gpd.GeoSeries = None,
clip_limit: None | int = 2,
consolidation_tolerance: None | float = None,
clip_limit: None | float | int = 2,
consolidation_tolerance: None | float | int = None,
) -> tuple[np.ndarray]:
"""
Returns average geometry.
Expand All @@ -117,20 +117,20 @@ def voronoi_skeleton(
Polygon enclosed by ``lines``.
snap_to : None | gpd.GeoSeries = None
Series of geometries that shall be connected to the skeleton.
max_segment_length: int = 1
max_segment_length: float | int = 1
Additional vertices will be added so that all line segments
are no longer than this value. Must be greater than 0.
buffer : None | float | int = None
Optional custom buffer distance for dealing with Voronoi infinity issues.
secondary_snap_to : None | gpd.GeoSeries = None
Fall-back series of geometries that shall be connected to the skeleton.
clip_limit : None | int = 2
clip_limit : None | float | int = 2
Following generation of the Voronoi linework, we clip to fit inside the polygon.
To ensure we get a space to make proper topological connections from the
linework to the actual points on the edge of the polygon, we clip using a
polygon with a negative buffer of ``clip_limit`` or the radius of
maximum inscribed circle, whichever is smaller.
consolidation_tolerance : None | float = None
consolidation_tolerance : None | float | int = None
Tolerance passed to node consolidation within the resulting skeleton.
If ``None``, no consolidation happens.

Expand Down Expand Up @@ -291,7 +291,7 @@ def _as_parts(edgelines: np.ndarray) -> np.ndarray:


def _consolidate(
edgelines: np.ndarray, consolidation_tolerance: float | int
edgelines: np.ndarray, consolidation_tolerance: None | float | int
) -> np.ndarray:
"""Return ``edgelines`` from consolidated nodes, if criteria met."""
if consolidation_tolerance and edgelines.shape[0] > 0:
Expand Down
92 changes: 71 additions & 21 deletions sgeop/simplify.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,39 +469,89 @@ def get_solution(group, roads):


def simplify_network(
roads,
roads: gpd.GeoDataFrame,
*,
max_segment_length=1,
min_dangle_length=20,
clip_limit: int = 2,
simplification_factor=2,
consolidation_tolerance=10,
artifact_threshold=None,
artifact_threshold_fallback=None,
area_threshold_blocks=1e5,
isoareal_threshold_blocks=0.5,
area_threshold_circles=5e4,
isoareal_threshold_circles_enclosed=0.75,
isoperimetric_threshold_circles_touching=0.9,
eps=1e-4,
exclusion_mask=None,
predicate="intersects",
):
"""
max_segment_length: float | int = 1,
min_dangle_length: float | int = 20,
clip_limit: float | int = 2,
simplification_factor: float | int = 2,
consolidation_tolerance: float | int = 10,
artifact_threshold: None | float | int = None,
artifact_threshold_fallback: None | float | int = None,
area_threshold_blocks: float | int = 1e5,
isoareal_threshold_blocks: float | int = 0.5,
area_threshold_circles: float | int = 5e4,
isoareal_threshold_circles_enclosed: float | int = 0.75,
isoperimetric_threshold_circles_touching: float | int = 0.9,
eps: float = 1e-4,
exclusion_mask: None | gpd.GeoSeries = None,
predicate: str = "intersects",
) -> gpd.GeoDataFrame:
"""Top-level workflow for simplifying networks. The input raw road network data is
first preprocessed (topological corrections & node consolidation) before two
iterations of artifact detection and simplification. For further information on
face artifact detection and extraction see :cite:`fleischmann2023`.
Comment on lines +490 to +493
Copy link
Contributor

Choose a reason for hiding this comment

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

This will probably need way more but I am happy to craft that documentation at some point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ya, I intended this a first minimal pass to get the process started. With that in mind, is there any more of an explanation to be added in this iteration?


Parameters
----------

clip_limit : int = 2
roads : geopandas.GeoDataFrame
Raw road network data.
max_segment_length : float | int = 1
Additional vertices will be added so that all line segments
are no longer than this value. Must be greater than 0.
Used in multiple internal geometric operations.
min_dangle_length : float | int
The threshold for determining if linestrings are dangling slivers to be
removed or not.
clip_limit : float | int = 2
Following generation of the Voronoi linework in ``geometry.voronoi_skeleton()``,
we clip to fit inside the polygon. To ensure we get a space to make proper
topological connections from the linework to the actual points on the edge of
the polygon, we clip using a polygon with a negative buffer of ``clip_limit``
or the radius of maximum inscribed circle, whichever is smaller.
simplification_factor : float | int = 2
The factor by which singles, pairs, and clusters are simplified.
This value is multiplied by ``max_segment_length``.
Copy link
Contributor

Choose a reason for hiding this comment

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

This can take a bit to understand. In my head it is the vice versa, the max_segment_length is multiplied by this factor to get the simplification epsilon.

consolidation_tolerance : float | int = 10
Tolerance passed to node consolidation within the
``geometry.voronoi_skeleton()``.
artifact_threshold : None | float | int = None
First option threshold used to determine face artifacts.
Passed into ``artifacts.get_artifacts()``.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not clear.

When artifact_threshold is passed, we don't compute it from artifacts and just use the given value. This is useful for small networks where artifact detection may fail or become unreliable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, given artifacts.get_artifacts() and everything else mentioned here is essentially private, I am not sure public docstring should point to that.

artifact_threshold_fallback : None | float | int = None
Second option threshold used to determine face artifacts.
Copy link
Contributor

Choose a reason for hiding this comment

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

And this is used as a fallback. So we try to detect artifact threshold from the data and use that one but if that fails, we fall back to the value given here.

Passed into ``artifacts.get_artifacts()``.
area_threshold_blocks : float | int = 1e5
Areal theshold for block detection.
Passed into ``artifacts.get_artifacts()``.
isoareal_threshold_blocks : float | int = 0.5
Isoareal theshold for block detection.
See ``esda.shape.isoareal_quotient``.
Passed into ``artifacts.get_artifacts()``.
area_threshold_circles : float | int = 5e4
Areal theshold for circle detection.
Passed into ``artifacts.get_artifacts()``.
isoareal_threshold_circles_enclosed : float | int = 0.75
Isoareal theshold for enclosed circle detection.
See ``esda.shape.isoareal_quotient``.
Passed into ``artifacts.get_artifacts()``.
isoperimetric_threshold_circles_touching : float | int = 0.9
Isoperimetric theshold for enclosed circle touching.
See ``esda.shape.isoperimetric_quotient``.
Passed into ``artifacts.get_artifacts()``.
Copy link
Contributor

Choose a reason for hiding this comment

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

All these will eventually, on the public API need more explanation on what they do and why they exist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll create a ticket dedicated to that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

xref: #110

eps : float = 1e-4
Tolerance epsilon used in multiple internal geometric operations.
exclusion_mask : None | geopandas.GeoSeries = None
Polygons used to determine face artifacts to exclude from returned output.
Passed into ``artifacts.get_artifacts()``.
predicate : str = 'intersects'
The spatial predicate used to exclude face artifacts from returned output.
Passed into ``artifacts.get_artifacts()``.

Returns
-------

geopandas.GeoDataFrame
The final, simplified road network line data.
"""

roads = fix_topology(roads, eps=eps)
Expand Down