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

COMPAT: specify shuffle=False in dissolve #229

Merged
merged 3 commits into from
Jan 7, 2023

Conversation

jorisvandenbossche
Copy link
Member

On top of #228 (will rebase this once the other is merged)

Starting with dask 2022.9.1, groupby started to use shuffle by default with split_out>1, see dask/dask#9453

But doing that currently doesn't work (at least with the test case). It comes down to _meta_nonempty failing to be determined, an issue with renamed columns and then the GeoDataFrame constructor fails with that:

@meta_nonempty.register(geopandas.GeoDataFrame)
def _nonempty_geodataframe(x):
df = meta_nonempty_dataframe(x)
return geopandas.GeoDataFrame(df, geometry=x._geometry_column_name, crs=x.crs)

First, it raises "The CRS attribute of a GeoDataFrame without an active geometry column is not defined. Use GeoDataFrame.set_geometry to set the active geometry column.", and after removing crs=x.crs, it raises "ValueError: Unknown column geometry" because the groupby shuffle renamed the columns

(I remember we ran into similar issues before, but not fully sure anymore if we solved that / where this discussion is)

martinfleis
martinfleis previously approved these changes Jan 6, 2023
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.

Thanks! Feel free to merge #228 as well once happy, looks okay to me.

It seems to require some version-based solution.

@martinfleis martinfleis dismissed their stale review January 6, 2023 13:13

missed CI...

@jorisvandenbossche
Copy link
Member Author

Ah, indeed, the shuffle keyword was added in dask/dask#9302 with dask 2022.8.1

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