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

handling edge cases in preprocessing.FaceArtifacts #666

Merged
merged 2 commits into from
Nov 11, 2024
Merged

handling edge cases in preprocessing.FaceArtifacts #666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1e803a
resolve #664 - resolve #665
jGaboardi Nov 11, 2024
a47fd6f
all None attrs
jGaboardi Nov 11, 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
26 changes: 25 additions & 1 deletion momepy/preprocessing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1524,7 +1524,7 @@ def _euclidean_simplification(geometry, new_origin, new_destination):


class FaceArtifacts:
"""Identify face artifacts in street networks
"""Identify face artifacts in street networks.

For a given street network composed of transportation-oriented geometry containing
features representing things like roundabouts, dual carriegaways and complex
Expand Down Expand Up @@ -1579,6 +1579,12 @@ class FaceArtifacts:
10 POLYGON ((-744101.275 -1043738.053, -744103.80... 2.862871
12 POLYGON ((-744095.511 -1043623.478, -744095.35... 3.712403
17 POLYGON ((-744488.466 -1044533.317, -744489.33... 5.158554

Notes
-----
In purely synthetic scenarios where all calculated face artifact index values are
equal (e.g. a regular grid/lattice) a ``LinAlgError`` error will be raised. This
is virtually guaranteed to not happen in practice.
"""

def __init__(
Expand Down Expand Up @@ -1606,6 +1612,24 @@ def __init__(

# Store geometries as a GeoDataFrame
self.polygons = gpd.GeoDataFrame(geometry=polygons)

if self.polygons.empty:
warnings.warn(
"Input roads could not not be polygonized. "
"Identification of face artifacts not possible.",
UserWarning,
stacklevel=2,
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
)
)
self.threshold = None
self.face_artifacts = None

I am not sure if we should not do this and maybe some other.

Copy link
Member Author

@jGaboardi jGaboardi Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So should we go ahead and assign all possible attributes as None?

  • kde
  • pdf
  • peaks
  • d_peaks
  • valleys
  • d_valleys
  • threshold
  • face_artifacts

Copy link
Member Author

Choose a reason for hiding this comment

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

I went ahead and pushed up all attr = None

self.kde = None
self.pdf = None
self.peaks = None
self.d_peaks = None
self.valleys = None
self.d_valleys = None
self.threshold = None
self.face_artifacts = None
return

if index == "circular_compactness":
self.polygons["face_artifact_index"] = np.log(
shape.minimum_bounding_circle_ratio(polygons) * polygons.area
Expand Down
12 changes: 12 additions & 0 deletions momepy/tests/test_preprocess.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,15 @@ def test_FaceArtifacts():

with pytest.raises(ValueError, match="'banana' is not supported"):
mm.FaceArtifacts(gdf, index="banana")

p1, p2, p3, p4 = Point(1, 0), Point(2, 0), Point(3, 0), Point(2, 1)
inverted_t = [LineString((p1, p2)), LineString((p2, p3)), LineString((p2, p4))]

with pytest.warns(
UserWarning,
match=(
"Input roads could not not be polygonized. "
"Identification of face artifacts not possible."
),
):
mm.FaceArtifacts(gpd.GeoDataFrame(geometry=inverted_t))
Loading