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.

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

Lasso update #16

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Lasso update #16

wants to merge 3 commits into from

Conversation

lachlan-git
Copy link
Collaborator

Minor Code and bug fixes optimised for compatibility with BayAreaMetro:
BayAreaMetro/Lasso#6 (comment)

@@ -1015,6 +1015,19 @@ def build_centroid_connectors(
"Missing taz node file, will use input taz polygon centroid"
)

if "county_gdf" not in self.__dict__.keys():
print('counhty not initialised')
self.county_gdf = gpd.read_file(r"\\corp.pbwan.net\us\CentralData\DCCLDA00\Standard\sag\projects\MTC\31000152\Network_Rebuild\LP_Local_folders\data\external\county_boundaries\county.shp")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like having this part in Ranch. Can we move this part to MTC's lasso?

@@ -1101,7 +1114,7 @@ def build_centroid_connectors(
"Missing maz node file, will use input maz polygon centroid"
)

maz_node_gdf = maz_polygon_gdf["geometry"].representative_centroids()
maz_node_gdf = maz_polygon_gdf["geometry"].centroid
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use representative_point() to force zone centroid to be within the polygon boundary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ill have to check because my version of geopandas did not have a representative point, I will have to fiddle with environment

@@ -1152,7 +1165,8 @@ def build_taz_drive_connector(
# geometries (not reference) - good intersections

taz_good_intersection_df = Roadway.get_nodes_in_zones(
node_two_geometry_df, taz_polygon_df
node_two_geometry_df.drop(columns=["index_left", "index_right"], errors="ignore"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this? If these two columns exist in the MTC workflow, I'd rather clean up the MTC data before calling this method. We should keep Ranch as generic as possible.

"\u001b[1;31mNameError\u001b[0m: name 'external_dir' is not defined"
]
}
],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why committing an error message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should Probably Roll Back that file

@@ -385,6 +385,9 @@ def generate_centroid_connectors_shape(zone_loading_node_df):
columns={"osm_node_id": "u", "model_node_id_x": "A", "model_node_id_y": "B"}
)

if ("A" not in new_cc_gdf.columns ) and ("B" not in new_cc_gdf.columns):
new_cc_gdf = new_cc_gdf.rename(columns={"X": "A", "Y": "B"})
Copy link
Collaborator

Choose a reason for hiding this comment

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

X/Y are the coordinates, A/B are the node IDs, what's the meaning of this rename?

@@ -680,7 +683,7 @@ def get_non_near_connectors(all_cc_link_gdf, num_connectors_per_centroid, zone_i
centroid = zone_cc_gdf.c_point.iloc[0]

# if the zone has less than 4 cc, keep all
if len(zone_cc_gdf) <= num_connectors_per_centroid:
if len(zone_cc_gdf) <= 4:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make more sense to change the value to num_connectors_per_centroid than hardcoding?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants