BUG: ndarrays pickled by 1.16 cannot be loaded by 1.15.4 and lower #12842

mattip · 2019-01-24T15:52:13Z

Fixes #12837
When pickling ndarrays, the pickle protocol adds the reconstructing function's modulename to the pickled string so it can be called. The function (which is_reconstruct) now lives in numpy.core._multiarray_umath, but in pre-1.16 numpy it was in numpy.core.multiarray. Also for scalar objects, the function scalar` moved.

The fix is very simple, changing the __module__ attribute of _reconstruct and scalar

In array_reduce_ex, used when the pickle protocol is 5, the function will be numpy.core.numeric._frombuffer, which will not exist on pre-1.16 numpy (added in commit 64a855f, Oct 2018). I don't think we can work around that. People who do not have that function will have to modify their numpy if they wish to import protocol-5 pickled ndarrays.

numpy/core/tests/test_regression.py

charris · 2019-01-24T17:17:42Z

Lots of test failures.

charris · 2019-01-24T17:54:48Z

There is a dedent function that does that, look at test_arrayprint.py for some examples. It might be better to separate the shell code from the call in any case.

mattip · 2019-01-24T18:16:23Z

3.5 fails because I used fstrings. Since the test is marked slow, and we only run slow on 3.6, I will skip 3.5 altogether.

3.6 is failing tests that check stdout, since the output is getting clobbered with "Coverage.py warning: --include is ignored because --source is set (include-ignored)". Did something change in coverage? Is this just on this PR or elsewhere too?

charris · 2019-01-24T18:33:26Z

Cannot backport if it uses fstrings.

mattip · 2019-01-25T10:59:27Z

Removed fstrings, rebased off master, but the test still causes problems on travis. It seems creating a venv via pytohn3 -venv and installing things into it somehow messes up the current environment on travis, azure seems OK with it.

I am not sure why the numpy/distutils/tests/test_exec_command.py test is failing, see PR #12849

mattip · 2019-01-25T12:35:16Z

numpy/core/tests/test_regression.py

+    @pytest.mark.skipif(not sys.platform.startswith('linux'),
+                        reason="linux only")
+    @pytest.mark.skipif(sys.version_info < (3,4), reason="lacks venv")
+    @pytest.mark.skip("venv on travis is flakey")


Skip the test entirely for now, maybe someday re-enable. It passes when run alone (--k test_pickle_across_versions) which gives me confidence that the actual fix works

How about just look at the pickle header?

Hmm, OK, looks like pickle itself determines the module, but it must be in the header somewhere. I wonder if the pickle can simply be searched for the module name? For instance

In [30]: a = array(1) In [31]: pickle.dumps(a, protocol=4) Out[31]: b'\x80\x04\x95\x96\x00\x00\x00\x00\x00\x00\x00\x8c\x1cnumpy.core._multiarray_umath\x94\x8c\x0c_reconstruct\x94\x93\x94\x8c\x05numpy\x94\x8c\x07ndarray\x94\x93\x94K\x00\x85\x94C\x01b\x94\x87\x94R\x94(K\x01)h\x03\x8c\x05dtype\x94\x93\x94\x8c\x02i8\x94K\x00K\x01\x87\x94R\x94(K\x03\x8c\x01<\x94NNNJ\xff\xff\xff\xffJ\xff\xff\xff\xffK\x00t\x94b\x89C\x08\x01\x00\x00\x00\x00\x00\x00\x00\x94t\x94b.'

Looking at that, I wonder why the umath module is there? Maybe from the ndarray __reduce__ method? AFAICT that method should not be called, being replaced by __reduce_ex__. I think we should look at both those methods.

We should also test the different protocols.

If you really want to test across versions, could also pickle some ndarray in an earlier version, get the hash of the result, and check that it agrees with the hash of the same data pickled with a latter version. The "correct" hashes can be stored in the test.

For instance with current master and previous array

In [43]: hashlib.sha256(s).hexdigest() Out[43]: 'cce278984d68606f0ceb4d23c298ebd8abe944adba1bc6b161f16c180e785fc3'

The array_reduce function does look buggy, but it is unlikely it gets called unless some ndarray subclass does it explicitly.

I wonder why the umath module is there

The module is _multiarray_umath.

I will write a test that checks the bytestring for the correct module name in all protocols. Tieing ourselves to the exact pickle bytestring or hash seems too restrictive going forward.

charris · 2019-01-25T20:17:16Z

There is also some ufunc pickling stuff in numpy/core/__init__.py that might could have problems.

charris · 2019-01-25T21:15:07Z

The ufunc pickles look OK.

charris · 2019-01-26T00:13:56Z

Is _reconstruct the only problem?

mattip · 2019-01-26T20:54:27Z

_reconstruct for ndarrays and scalar for scalars both need their __module__ adjusted

charris · 2019-01-27T17:03:22Z

azure-pipelines.yml

@@ -14,7 +14,7 @@ jobs:
           docker pull i386/ubuntu:bionic
           docker run -v $(pwd):/numpy i386/ubuntu:bionic /bin/bash -c "cd numpy && \
           apt-get -y update && \
-           apt-get -y install python3.6-dev python3-pip locales && \
+           apt-get -y install python3.6-dev python3-venv python3-pip locales && \


No longer need python3-venv?

True, but on the other hand it couldn't hurt and makes this environment consistent with the other linux ones one travis

Come, come, you could install lots of things that don't hurt, but that is not a reason to do it.

ok, removing

charris · 2019-01-27T17:05:01Z

LGTM, one possible nit.

charris · 2019-01-27T18:43:42Z

Thanks Matti.

mattip added 00 - Bug component: numpy._core 09 - Backport-Candidate PRs tagged should be backported labels Jan 24, 2019

mattip added this to the 1.16.1 release milestone Jan 24, 2019

charris reviewed Jan 24, 2019

View reviewed changes

numpy/core/tests/test_regression.py Outdated Show resolved Hide resolved

mattip force-pushed the backward-pickle branch 3 times, most recently from 06adfdd to abc604f Compare January 25, 2019 10:48

mattip mentioned this pull request Jan 25, 2019

TEST: remove COV_* from environment before testing Popen #12849

Closed

mattip commented Jan 25, 2019

View reviewed changes

mattip force-pushed the backward-pickle branch from abc604f to a21bc6a Compare January 27, 2019 07:14

mattip mentioned this pull request Jan 27, 2019

azure-pipelines broken for 32-bit compiles. #12856

Closed

mattip force-pushed the backward-pickle branch from a21bc6a to 74be893 Compare January 27, 2019 13:13

mattip closed this Jan 27, 2019

mattip reopened this Jan 27, 2019

mattip force-pushed the backward-pickle branch from 74be893 to 4b9d20a Compare January 27, 2019 14:27

charris reviewed Jan 27, 2019

View reviewed changes

BUG: ndarrays pickled by 1.16 cannot be loaded by 1.15.4 and lower

2b5b009

mattip force-pushed the backward-pickle branch from 4b9d20a to 2b5b009 Compare January 27, 2019 18:05

charris merged commit 5c370cb into numpy:master Jan 27, 2019

charris mentioned this pull request Jan 27, 2019

BUG: ndarrays pickled by 1.16 cannot be loaded by 1.15.4 and lower #12869

Merged

charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jan 27, 2019

charris removed this from the 1.16.1 release milestone Jan 27, 2019

mattip mentioned this pull request Jan 29, 2019

ImportError: numpy.core._multiarray_umath failed to import #11871

Closed

mattip deleted the backward-pickle branch May 20, 2019 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: ndarrays pickled by 1.16 cannot be loaded by 1.15.4 and lower #12842

BUG: ndarrays pickled by 1.16 cannot be loaded by 1.15.4 and lower #12842

mattip commented Jan 24, 2019 •

edited by charris

charris commented Jan 24, 2019

charris commented Jan 24, 2019

mattip commented Jan 24, 2019 •

edited

charris commented Jan 24, 2019

mattip commented Jan 25, 2019

mattip Jan 25, 2019

charris Jan 25, 2019

charris Jan 25, 2019

charris Jan 25, 2019

charris Jan 25, 2019

charris Jan 25, 2019

charris Jan 25, 2019

charris Jan 25, 2019

mattip Jan 26, 2019

charris commented Jan 25, 2019

charris commented Jan 25, 2019

charris commented Jan 26, 2019

mattip commented Jan 26, 2019

charris Jan 27, 2019

mattip Jan 27, 2019

charris Jan 27, 2019

mattip Jan 27, 2019

charris commented Jan 27, 2019

charris commented Jan 27, 2019

BUG: ndarrays pickled by 1.16 cannot be loaded by 1.15.4 and lower #12842

BUG: ndarrays pickled by 1.16 cannot be loaded by 1.15.4 and lower #12842

Conversation

mattip commented Jan 24, 2019 • edited by charris

charris commented Jan 24, 2019

charris commented Jan 24, 2019

mattip commented Jan 24, 2019 • edited

charris commented Jan 24, 2019

mattip commented Jan 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Jan 25, 2019

charris commented Jan 25, 2019

charris commented Jan 26, 2019

mattip commented Jan 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Jan 27, 2019

charris commented Jan 27, 2019

mattip commented Jan 24, 2019 •

edited by charris

mattip commented Jan 24, 2019 •

edited