Skip to content

Ignore Python 3.11's default __getstate__ implementation#396

Merged
Theelx merged 1 commit intojsonpickle:mainfrom
encukou:has_own_getstate
Jun 23, 2022
Merged

Ignore Python 3.11's default __getstate__ implementation#396
Theelx merged 1 commit intojsonpickle:mainfrom
encukou:has_own_getstate

Conversation

@encukou
Copy link
Copy Markdown
Contributor

@encukou encukou commented Jun 22, 2022

Fixes: #395
Python 3.11 adds a default __getstate__ to all objects, which breaks jsonpickle. See the issue for details.

If jsonpickle wants to keep its current behavior (and not have some objects that serialized as py/reduce in 3.10 turn into py/objectwithpy/state`), the pickler can ignore the default method.

With this the tests should pass on 3.11. (I didn't try with all test dependencies, though.)

Please consider this a draft/suggestion. I'm not a pickling expert by any means.

@AdamWill
Copy link
Copy Markdown

This makes all the tests in @hroncok 's reduced "no-compilation" set pass at least, confirmed. I'll try a mock package build to see if all tests pass. Not sure if this is the correct/best fix, but it does seem to do the job at least. Thanks.

@AdamWill
Copy link
Copy Markdown

Looks good in a package build too, where we run PYTHON=python3 make test. It gives:

============ 342 passed, 13 skipped, 3 xpassed, 7 warnings in 1.54s ============

@Theelx
Copy link
Copy Markdown
Contributor

Theelx commented Jun 22, 2022

Thanks for this PR. Since we're going to be releasing jsonpickle 3.0.0 soon anyway, this would be a good fit for inclusion since it's a breaking change. Is it possible to deserialize objects on this branch that were encoded with 2.2.0, or do we need a special helper function for that?

@Theelx
Copy link
Copy Markdown
Contributor

Theelx commented Jun 22, 2022

Ok, it looks good to me. When I pickle an object from 2.2.0 and unpickle in your fork, it returns the correct deserialized object. So, there should be no need for a helper function. I'm all good with this, just waiting on a second opinion from @davvid.

@AdamWill
Copy link
Copy Markdown

Yeah, that makes sense based on what the change does, I wouldn't expect there to be compatibility issues. This only changes pickler behaviour, and it should only change it such that on 3.11 it now behaves the way it used to with 3.10.

@Theelx
Copy link
Copy Markdown
Contributor

Theelx commented Jun 23, 2022

I'm going to merge this beause I like the implementation, and davvid can revert if needed. When do you guys need 3.11 support by @encukou @AdamWill @hroncok? There's some other work I'd like to get in before I release 3.0.0, but if y'all need this patch in a public release soon then that can be arranged (but it'd probably be 2.2.1 or 2.3.0).

@Theelx Theelx merged commit 5236ea2 into jsonpickle:main Jun 23, 2022
@davvid
Copy link
Copy Markdown
Member

davvid commented Jun 23, 2022

Thank you so much for making this happen. Really appreciate the attention to new Python versions.

@AdamWill
Copy link
Copy Markdown

AdamWill commented Jun 23, 2022

@Theelx it's not urgent to get it in a public release, I've already backported the patch for our downstream package build. As long as it's on track for the next upstream release, that's all good. Doing a new release will save other distributors a bit of work when they come to bump to Python 3.11, probably, but that's all. Thanks a lot!

@vstinner
Copy link
Copy Markdown

Nice fix :-)

@encukou encukou deleted the has_own_getstate branch June 24, 2022 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failures with Python 3.11

5 participants