-
Notifications
You must be signed in to change notification settings - Fork 13
Improve robustness of welding #159
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -229,11 +229,11 @@ def check_double_points(gdf, radius=0.001, id_column=None): | |
| return l_ids | ||
|
|
||
|
|
||
| def gdf_to_df(gdf): | ||
| def gdf_to_df(df): | ||
| """Converts a GeoDataFrame to a pandas.DataFrame by deleting the geometry | ||
| column.""" | ||
|
|
||
| df = pd.DataFrame(gdf[[col for col in gdf.columns if col != "geometry"]]) | ||
| if isinstance(df, gpd.GeoDataFrame): | ||
| df = df.drop(columns=df.geometry.name) | ||
|
|
||
| return df | ||
|
|
||
|
|
@@ -364,6 +364,9 @@ def weld_segments( | |
| gdf_line_net_new = _weld_segments( | ||
| gdf_line_net_new, gdf_line_gen, gdf_line_houses, debug_plotting | ||
| ) | ||
| if len(gdf_line_net_new) == 0: | ||
| gdf_line_net_new = gdf_line_net_last | ||
| break | ||
|
Comment on lines
+367
to
+369
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a comment when this might happen?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I remember correctly, I had a case where |
||
| logger.info("Welding lines... done") | ||
| return gdf_line_net_new | ||
|
|
||
|
|
@@ -586,6 +589,7 @@ def debug_plot(neighbours, color="red"): | |
| # Merge the MultiLineString into a single object | ||
| merged_line = linemerge(multi_line) | ||
| gdf_merged = gpd.GeoDataFrame(geometry=[merged_line], crs=crs) | ||
| gdf_merged = gdf_merged.explode() # Explode Multi- into LineStrings | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are other places in the code (e.g. specifically |
||
| debug_plot(neighbours) # Plot the segments before the merge | ||
| debug_plot(gdf_merged, color="orange") # ...and after the merge | ||
| gdf_line_net_new = pd.concat( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to rename this to something like
drop_geometry_data? In particular, as the new parameter name suggests that that also plainDataFrames(not onlyGeoDataFrames) are okay.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first my idea was to only write
df = gdf.drop(columns=gdf.geometry.name). But I thought it would be saver to check if the givenDataFrameis actually aGeoDataFrame. In fact, the previous version of that function actually works withDataFrames, because it would not change anything if no column named"geometry"is found.Therefore plain
DataFrameswere ok before, and are ok now. I feel like renaming the input to"df"is only logical, in that sense.I do not feel the need to rename the function, but I would not argue against it.