-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add compatibility for Dask 2021.06.0 #70
Conversation
Thanks! Since you're a core Dask team member, I'll take this suggestion as the authoritative response for what to do for dask <=2021.5.0 and >=2021.6.0. |
Yeah, sorry about the churn between the 2021.5.0 and 2021.6.0 releases. The changes here should provide compatibility for all but 2021.5.1 (it sounds like you're okay saying that's just a bad release). From what I can tell, it looks like the CI failures here are unrelated to the Dask changes. Is that the case? |
Thanks. Yes, I'm fine with ignoring 2021.05.1. I agree that the test failures seem unrelated, but I have no idea what is causing them. Something to do with pyarrow, numpy or pandas, presumably? |
Test failures are likely due to an old version of pyarrow getting installed. I'll try to fix. |
Seems like conda can't solve for geopandas and a recent pyarrow version when pointed only at defaults so I've had to add conda-forge. |
Thanks for the updates @philippjfr! For the remaining
issue, there are a couple of upstream |
Thanks for the updates @philippjfr! It looks like there are some Python version inconsistencies in CI builds now. I've made a first attempt at resolving those issues, though since I've not committed to this repo before I need approval for CI builds to run |
At this point all builds are passing other than the Python 3.7 macOS build, which is failing for some unknown reason. At this point I'm mostly mucking around in CI configuration that's not immediately related to the original |
Yeah, it's really not for you to worry about at this point. It just seems very difficult to get a working environment which includes geopandas (and therefore libgdal) and recent pyarrow, which works across platforms. Will maybe just disable the OSX test for now and ping the conda team about these issues again. |
Would spatialpandas tests be able to use a "geopandas-core" package that didn't depend on GDAL or libgdal? At one point @jorisvandenbossche indicated that "we could easily look into making fiona/gdal optional for geopandas, as it is not a hard requirement". I'd love to be able to use geopandas in situations like this without having the dependency issues of GDAL. I know this wouldn't be an immediate solution, but for the long term... |
The latest version of GeoPandas already doesn't hard-depend on fiona (as recently mentioned in #1 (comment)), so you could install it with pip (just with And for conda, I actually just looked into making a |
A first user of geopandas-base! ;) |
I assume you were a bit too fast. I only just merged the change right before, so it takes a bit before it is present on the conda channels. If you restart CI now, it should probably be better. |
The Python 3.7 on macOS are running 🎉 |
Well that came just at the right time, thanks @jorisvandenbossche! And @jrbourbeau, sorry you had to deal with the bottomless pit of tears and despair that comes with dealing with any environment with a dependency on libgdal. Will merge and then tag a dev release. |
No worries! I've got a couple of other small CI cleanup things I want to test out, but let's save that for a follow-up PR : ) |
from setuptools import find_packages, setup | ||
|
||
extras_require = { | ||
'tests': [ | ||
'codecov', | ||
'flake8', | ||
'geopandas', | ||
'hilbertcurve', | ||
'geopandas-base', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that geopandas-base is only for conda-forge, so this won't work for pip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, that's okay for now. I guess we should disallow pip install spatialpandas[test]
somehow but for development, testing and CI we recommend conda anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our build setup (pyctdev) does have ways to deal with x vs x-base or x-core differing between pip and conda, but I am not sure exactly how it handles that.
BTW, if you use strict channel priority, in my experience this doesn't give that many tears (+ avoiding the default channel if you need recent versions) |
In my experience that's true for most versions but some combinations of OSX and certain Python versions have still given me headaches recently. |
Thanks again everyone, spatialpandas 0.4.1 is now on the conda pyviz channel and PyPI and in due course should be up on conda-forge as well. |
Yes, thanks so much, everyone! Truly a team effort! Now I can get back to the Datashader release that this was blocking. |
This PR adds some compatibility updates for the
dask=2021.06.0
release. xref dask/dask#7743 (comment)cc @jbednar
Supersedes #69, xref #68 (comment)