holoviz / spatialpandas

Pandas extension arrays for spatial/geometric operations
BSD 2-Clause "Simplified" License
308 stars 24 forks source link

error when importing DaskGeoDataFrame #68

Closed giancastro closed 3 years ago

giancastro commented 3 years ago

Due to the Dask release v2021.05.1, PR https://github.com/dask/dask/pull/7503, the DaskGeoDataFrame importing was broken.

Screenshot from 2021-06-01 10-15-57

Only the make_meta and meta_nonempty functions were moved to the dispatch module, make_array_nonempty were not being imported directly from the source module.

Probably the previous spatialpandas releases will have the same problem.

jakirkham commented 3 years ago

Please see this comment ( https://github.com/dask/dask/pull/7503#issuecomment-852084344 )

DahnJ commented 3 years ago

Might be relevant: This is how dask-geopandas has dealt with the change: https://github.com/geopandas/dask-geopandas/pull/47

giancastro commented 3 years ago

Hey @philippjfr / @jonmmease, are you following the discussions on conda-envs repo and the new PR on the Dask repo?

jakirkham commented 3 years ago

Yes it would be great to have your feedback. Want to figure out the right way forward. PR ( https://github.com/dask/dask/pull/7743 ) is relevant

jrbourbeau commented 3 years ago

Yeah, definitely let us know if you have any feedback on that PR. We're planning to release in the next couple of days and want to make sure we include fixes for any issues you're running into.

jrbourbeau commented 3 years ago

Just a heads up that https://github.com/dask/dask/pull/7743 has been merged into dask. This should hopefully resolve any issues that popped up with the 2021.05.1 release. We're planning a release tomorrow which will include the fix in https://github.com/dask/dask/pull/7743. Let us know if there are any lingering issues for spatialpandas

philippjfr commented 3 years ago

Thanks for all the updates here. I guess I'm somewhat unclear on any actions we should take? As far as I understand this will now be fixed in new versions of dask. So the only real actions we could take is update the repodata in defaults and conda-forge to exclude the affected versions. Am I misunderstanding and is that even worth doing?

jrbourbeau commented 3 years ago

You could definitely update package repodata (or just say "2021.05.1 was a bad release, don't use it"). Additionally, if you get a moment to run the spatialpandas test suite against the main branch of dask it would be great to see whether or not any changes need to be made in spatialpandas to add support the upcoming dask release (we moved a new objects around, so we may need to change some imports)

jbednar commented 3 years ago

Thanks, everyone, for the suggestions. I'm trying to release Datashader but I'm stymied by SpatialPandas being broken, which breaks Datashader's tests. I've tried to follow the discussion and suggestions here and in https://github.com/dask/dask/pull/7503 and https://github.com/dask/dask/pull/7743, but it's a complete nightmare!

To simplify things, let's accept @jrbourbeau's suggestion that "dask 2021.05.1 was a bad release, don't use it", i.e., let's assume no support for 202105.1. Having done so, neither our original code nor the code in PR #69 will work, though, because the original one imports from utils, no longer valid in 5.x, while #69 imports from dispatch, which wasn't present in 5.x. The old code:

$ python -c 'import dask; print(dask.__version__); from dask.dataframe.utils import make_meta, meta_nonempty, make_array_nonempty'
2021.05.0
$ python -c 'import dask; print(dask.__version__); from dask.dataframe.utils import make_meta, meta_nonempty, make_array_nonempty'
2021.05.1
ImportError: cannot import name 'make_array_nonempty' from 'dask.dataframe.utils' (/Users/jbednar/dask/dask/dataframe/utils.py)
$ python -c 'import dask; print(dask.__version__); from dask.dataframe.utils import make_meta, meta_nonempty, make_array_nonempty'
2021.06.0
ImportError: cannot import name 'make_array_nonempty' from 'dask.dataframe.utils' (/Users/jbednar/dask/dask/dataframe/utils.py)

Code from PR #69:

$ python -c 'import dask; print(dask.__version__); from dask.dataframe.dispatch import make_meta, meta_nonempty ; from dask.dataframe.extensions import make_array_nonempty'
2021.05.0
ModuleNotFoundError: No module named 'dask.dataframe.dispatch'
$ python -c 'import dask; print(dask.__version__); from dask.dataframe.dispatch import make_meta, meta_nonempty ; from dask.dataframe.extensions import make_array_nonempty'
2021.05.1
$ python -c 'import dask; print(dask.__version__); from dask.dataframe.dispatch import make_meta, meta_nonempty ; from dask.dataframe.extensions import make_array_nonempty'
2021.06.0

I'm pretty sure there is no other place these utilities were available that is valid both for 5.0 and 6.0. Ok, fine, we can put in a try/except, first looking in dispatch, and falling back to utils, which will presumably fix the import issues on at least 5.0 and 6.0. It's ugly that that's the only apparent solution, but fine.

What I really can't tell is whether that is sufficient, from the discussion here and in PR https://github.com/geopandas/dask-geopandas/pull/50/files replacing make_meta with make_meta_dispatch, PR https://github.com/rapidsai/cudf/pull/8368/files replacing make_meta_obj with make_meta and make_meta_util with make_meta , and comment https://github.com/dask/dask/pull/7503#issuecomment-852084344 suggesting replacing make_meta with make_meta_util. How are any of these utilities meant to be used?

@jsignell, it looks like your changes in https://github.com/dask/dask/pull/7503 were the original stirring of this hornet's nest, so could you perhaps weigh in with what you think the downstream libraries like cudf, dask-geopandas, and spatialpandas are supposed to do for compatibility with dask<=5.0 and dask>=6.0?

jsignell commented 3 years ago

Yeah there were two big shuffles centering on make_meta_* and it was definitely unfortunate that they landed in the same release. I see that @jrbourbeau has opened a compatibility PR - sorry for the pain.

jbednar commented 3 years ago

Thanks! As long as you and he agree that his proposed changes are good, I'm happy! :-)