Skip to content

Fix non-empty meta when reading from shapefiles #305

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

Merged
merged 5 commits into from
Aug 21, 2024

Conversation

fbunt
Copy link
Contributor

@fbunt fbunt commented Aug 5, 2024

Closes #302

@fbunt
Copy link
Contributor Author

fbunt commented Aug 5, 2024

I added tests for the other reader types to be safe but I can pull them out if needed.

@fbunt
Copy link
Contributor Author

fbunt commented Aug 20, 2024

Could someone review this?

@martinfleis
Copy link
Member

Sorry for delays! This looks good. Can you also add a test that verifies that the erroneous line you had in #302 indeed works now?

@fbunt
Copy link
Contributor Author

fbunt commented Aug 20, 2024

Sure!

@fbunt
Copy link
Contributor Author

fbunt commented Aug 21, 2024

I added a test module for IO integration tests. Let me know if this is overkill.

Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

Fine with me, thanks!

@martinfleis
Copy link
Member

Actually, the minimal env does not have pyogrio. Can you add skipif to avoid the failure we see now?

@fbunt
Copy link
Contributor Author

fbunt commented Aug 21, 2024

Fixed

@martinfleis martinfleis merged commit 19e0110 into geopandas:main Aug 21, 2024
13 checks passed
@fbunt fbunt deleted the issue-302 branch August 21, 2024 18:50
@fbunt fbunt restored the issue-302 branch August 21, 2024 18:51
@fbunt fbunt deleted the issue-302 branch August 21, 2024 18:51
@fbunt
Copy link
Contributor Author

fbunt commented Sep 23, 2024

Is it possible to do a patch release to get this out there?

@jorisvandenbossche
Copy link
Member

@fbunt will look into doing a small bug-fix release tomorrow!

@fbunt
Copy link
Contributor Author

fbunt commented Sep 23, 2024

Thanks!

@jorisvandenbossche
Copy link
Member

FWIW, I was wondering if this should be considered a bug in dask-expr (that their set_index should not pass such meta object to pandas' set_index), but so dask does document _meta as always being empty (https://docs.dask.org/en/stable/dataframe-design.html?highlight=_meta#metadata). So regardless of this fix being a workaround for what dask-expr set_index does, creating empty meta objects seems to be what is expected.

@fbunt and thanks for the fix!

@fbunt
Copy link
Contributor Author

fbunt commented Sep 24, 2024

Yeah it's tricky. I wasn't sure if that's how the rest of the _meta machinery works in dask/dask-expr.

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.

spatial_shuffle fails when loading from a shapefile
3 participants