Uh oh!
There was an error while loading. Please reload this page.
Fix cloudpickle incompatibilities on early Python 3.5 versions - #361
Conversation
Codecov Report
@@ Coverage Diff @@## master #361 +/- ##
=======================================
Coverage 92.95% 92.95% =======================================
Files 2 2 Lines 809 809 Branches 164 164 =======================================
Hits 752 752 Misses 29 29 Partials 28 28 Continue to review full report at Codecov.
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
da2631e to
7996513Comparepierreglaser
commented
Apr 28, 2020
rebased. |
…le into cloudpickle-py350
ogrisel
commented
Apr 28, 2020
Maybe it's not worth fighting with the CI too much to test Python 3.5.0... I will test this PR one more time locally. |
pierreglaser
commented
Apr 28, 2020
Cool. |
ogrisel
commented
Apr 28, 2020
How did you install Python 3.5.0 to run your tests? From source? I cannot find it with conda. |
pierreglaser
commented
Apr 28, 2020
let me know when if it works locally - if so, I'll clean up the CI additions in this PR. |
Using |
ogrisel
commented
Apr 28, 2020
Ok I managed to run the tests of this PR successfully using python:3.5.0 from docker. |
ogrisel
commented
Apr 28, 2020
From your last CI run: That's an interesting error message ;) I am not sure what it means. Anyways, feel free to rollback the CI changes and merge this PR to make the release. |
pierreglaser
commented
Apr 28, 2020
Great. I'm quickly testing 3.5.1 and 3.5.2 while I'm at it, and i'll merge. |
ogrisel
commented
Apr 28, 2020
Actually I made a mistake when testing with Python 3.5.0. I had forgotten to install the typing_extensions package. If I do so I get the following failure: ___________________________________________________________________ CloudPickleTest.test_generic_extensions ____________________________________________________________________
self = <tests.cloudpickle_test.CloudPickleTest testMethod=test_generic_extensions>
deftest_generic_extensions(self):
typing_extensions = pytest.importorskip('typing_extensions')
objs = [
typing_extensions.Literal,
typing_extensions.Final,
typing_extensions.Literal['a'],
typing_extensions.Final[int],
]
for obj in objs:
depickled_obj = pickle_depickle(obj, protocol=self.protocol)
> assert depickled_obj == obj
tests/cloudpickle_test.py:2137: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ /usr/local/lib/python3.5/site-packages/typing_extensions.py:653: in __eq__
ifnotisinstance(other, Literal):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = typing_extensions.Literal, obj = typing_extensions.Literal
def__instancecheck__(self, obj):
> raise TypeError("Literal cannot be used with isinstance().")
E TypeError: Literal cannot be used with isinstance().
/usr/local/lib/python3.5/site-packages/typing_extensions.py:624: TypeError
|
pierreglaser
commented
Apr 28, 2020
What kind of can of worms is this..... |
TypeVar objects are not weakreferable in early Python 3.5
pierreglaser
commented
Apr 28, 2020
The test suite passes locally on Let's sleep on it until tomorrow morning and then release. |
I confirm the tests now pass on my local 3.5.0. I will test each other 3.5.x version with docker and then review the diff.
|
ogrisel
commented
Apr 29, 2020
For your information, here is the command line I use to run the tests with docker: docker run --rm -v `pwd`:/io -ti python:3.5.2 \
bash -c "cd /io/ && pip install -e . psutil pytest typing_extensions && pytest -v" |
ogrisel
left a comment
There was a problem hiding this comment.
I think we should restore TypeVar tracking, at least for recent Python versions (>= 3.6).
Uh oh!
There was an error while loading. Please reload this page.
| name, *constraints, bound=bound, | ||
| covariant=covariant, contravariant=contravariant | ||
| ) | ||
| return _lookup_class_or_track(class_tracker_id, tv) |
There was a problem hiding this comment.
Why don't you track TypeVar defintions anymore? This seems unrelated to the change to support early 3.5.x.
Doesn't this change break ant test?
Edit: I see you removed test_pickle_dynamic_typevar_tracking.
There was a problem hiding this comment.
Also won't this break unpickling objects pickled with cloudpickle 1.4.0?
There was a problem hiding this comment.
TyperVar instances are not weakreferable in Python 3.5.3..
There was a problem hiding this comment.
Also won't this break unpickling objects pickled with cloudpickle 1.4.0?
We can always restore the class_tracker_id for backward compat.
There was a problem hiding this comment.
TyperVar instances are not weakreferable in Python 3.5.3..
I re-ran the tests with old Python 3.5.x and they pass...
There was a problem hiding this comment.
Hum you are right they fail just for 3.5.3 ...
There was a problem hiding this comment.
ok let me do a workaround for 3.5.3.
ogrisel
commented
Apr 29, 2020
Ok I re-enabled dynamic |
pierreglaser
commented
Apr 29, 2020
I'm not sure we can. See #361 (comment) |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ogrisel
left a comment
There was a problem hiding this comment.
LGTM now! Thanks very much @pierreglaser.
Closes#360 .
cloudpickle 1.4.0is not compatible with earlyPython 3.5versions.This should fix it.
Note that I did not set up any CI for
Python 3.5.[0-2], I simply tested it on my machine using freshcondaenvs.@vedran If you have some time, could you tell me if this branch fixes the problems that made you create #360?
I would be tempted to release a bugfix version by tonight since this bug completely breaks
cloudpickleonPython 3.5.