Import Error on Python 3.8

civisanalytics / civis-python

Civis API Python Client

BSD 3-Clause "New" or "Revised" License

34 stars 26 forks source link

Import Error on Python 3.8 #329

Closed channiemills closed 4 years ago

channiemills commented 5 years ago

There appears to be an incompatibility with civis python client 1.11.0 and python 3.8. I get a type error from the external library cloudpickle when importing the civis python client.

Full stack trace:

elsander commented 4 years ago

@stephen-hoover Do you know if there's a specific reason why cloudpickle is pinned to below version 1? I think this is probably the blocker for python 3.8.

The current version requirements are:

cloudpickle>=0.2,<=1 ; python_version != '3.4'
cloudpickle<1.2 ; python_version == '3.4'

stephen-hoover commented 4 years ago

IIRC, it's only because we haven't tested on cloudpickle 2 (it wasn't out yet when we set that version requirement) and don't know if any API-breaking changes affect us.

elsander commented 4 years ago

Oh wow, I just misread that it's <=1, not <1. I don't think version 2 is out yet. I'll dig into this more. We should be supporting 3.8 in our next release if possible.

stephen-hoover commented 4 years ago

This looks related to https://github.com/cloudpipe/cloudpickle/issues/266 . It sounds like it's fixed in their newest release.

elsander commented 4 years ago

Yeah, when I updated joblib and cloudpickle in my 3.8 dev environment, the issue went away. However, it looks like pyarrow is not currently compatible with 3.8: https://arrow.apache.org/docs/python/install.html

So I think the path forward on the import error is to have stricter requirements on the joblib and cloudpickle versions. We can restrict the python versions that try to install feather for now, and update it once it's available. It's annoying that a user who is keeping their python updated would end up with an older set of features (csv rather than feather for civisml), but I don't think we can fix it on our end.

elsander commented 4 years ago

Updating the dependencies makes it installable, but a lot of tests fail. This may be a somewhat involved update.

jacksonllee commented 4 years ago

To clarify, are we updating the version requirements for cloudpickle and joblib at civis-python in any way for the next civis-python release? In CivisML upgrade work in progress (and likely upcoming changes at datascience-python), I'm testing with cloudpickle v1.2.2 and joblib v0.14.1 in Python 3.7, and no issues so far. I was wondering if civis-python can remove the upper version bound for joblib.

elsander commented 4 years ago

We try to keep an upper bound on requirements at the major version, or minor version if the library only really makes minor version releases (like joblib). I can make a quick PR to relax those upper bounds to include new versions, but I don't want to remove the cap entirely. We're not going to address the Python 8 compatibility issue in this version release, though, since we need to make the release soon (possibly today). Are there any other library versions you'd need updated for CivisML?

jacksonllee commented 4 years ago

If a new release of civis-python is coming this week, allowing joblib 0.14.x is what CivisML (and datascience-python) would need. Thank you for the help!

mheilman commented 4 years ago

I think we can close this because of #349.