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

Always write fid index of Node and Edge tables #995

Merged
merged 3 commits into from
Jan 26, 2024
Merged

Always write fid index of Node and Edge tables #995

merged 3 commits into from
Jan 26, 2024

Conversation

visr
Copy link
Member

@visr visr commented Jan 26, 2024

We are using GeoDataFrame.to_file to write these tables to GeoPackage. By default GeoPandas only writes the index if it has a name. We don't want to rely on users naming indices, so this sets the name and ensure it is written.

I ran into this since I noticed the model.network.edge.df.index (by default 0-based) did not correspond to the 1-based fid column that GeoPandas writes.

https://geopandas.org/en/stable/docs/reference/api/geopandas.GeoDataFrame.to_file.html


gdf.to_file(path, layer=self.tablename(), driver="GPKG")
gdf.to_file(path, layer=self.tablename(), driver="GPKG", index=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly out of curiosty: What happens if index=True but no name is provided, does it error?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oef, then it writes the index to an index column, and still adds a 1-based autoincrement fid.
This is the trivial model, which has edge_id 9 and 11 only.
image

@visr visr merged commit 8a9a928 into main Jan 26, 2024
19 checks passed
@visr visr deleted the write-index branch January 26, 2024 15:28
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