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

BUG: fix writing fids to e.g. GPKG file with use_arrow #511

Conversation

theroggy
Copy link
Member

@theroggy theroggy commented Dec 20, 2024

resolves #510

@theroggy theroggy changed the title TST: add test to specify fid to be written to GPKG file BUG: fix writing fids to e.g. GPKG file with use_arrow Dec 20, 2024
pyogrio/_io.pyx Outdated Show resolved Hide resolved
@theroggy theroggy marked this pull request as draft December 20, 2024 11:58
@theroggy
Copy link
Member Author

I put the PR as draft while waiting for feedback on the comment here: OSGeo/gdal#11527 (comment)

@theroggy theroggy marked this pull request as ready for review December 21, 2024 08:35
pyogrio/_io.pyx Outdated Show resolved Hide resolved
pyogrio/_io.pyx Outdated Show resolved Hide resolved
pyogrio/_io.pyx Outdated Show resolved Hide resolved
output_gdf = read_dataframe(path, fid_as_index=True, use_arrow=use_arrow)
output_gdf = output_gdf.reset_index()

# pyogrio always sets "fid" as index name with `fid_as_index`
Copy link
Member

Choose a reason for hiding this comment

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

Question: should we be preserving the original custom name when uisng fid_as_index?

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been suggested/requested in this comment: #287 (comment)

I don't mind, but it will be a breaking change...

Copy link
Member

Choose a reason for hiding this comment

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

Probably fine to leave that for a different PR to deal with that specifically.

Copy link
Member

@brendan-ward brendan-ward left a comment

Choose a reason for hiding this comment

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

Thanks @theroggy

@brendan-ward brendan-ward merged commit 6d6ebfb into geopandas:main Dec 27, 2024
22 checks passed
@theroggy theroggy deleted the TST-add-test-to-specify-fid-to-be-written-to-GPKG-file branch December 27, 2024 18:38
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.

BUG: write_dataframe to GPKG with an "fid" column gives an error if use_arrow=True
3 participants