Skip to content
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

Fix obscure pickling bug affecting Python 3.11 #2426

Merged
merged 3 commits into from
Jun 12, 2023
Merged

Conversation

rjgildea
Copy link
Contributor

This primarily affected multiprocessing in dials.integrate on Windows using Python 3.11.

The Executor base class is defined in C++ with .enable_pickling to enable lightweight pickle support. This is then subclassed e.g. for the IntegratorExecutor class, where a __getinitargs__() method is defined to provide support for pickling. For Python <= 3.10 this works just fine. However, bpo-26579 adds a default object.__getstate__() method in Python 3.11 which then triggers the Incomplete pickle support (__getstate_manages_dict__ not set) error message in Boost.Python. Adding __getstate_manages_dict__ = 1 to these subclasses appears to workaround the problem.

For example, the code print(f"{executor.__getstate__=}") on python 3.11 gives:

executor.__getstate__=<built-in method __getstate__ of IntegratorExecutor object at 0x7f52eefbf9c0>

while on python 3.10 raises an AttributeError:

AttributeError: Please report this error to dials-support@lists.sourceforge.net: 'IntegratorExecutor' object has no attribute '__getstate__'. Did you mean: '__setattr__'?

This primarily affected multiprocessing in dials.integrate on Windows using
Python 3.11.

The Executor base class is defined in C++ with .enable_pickling to enable
lightweight pickle support. This is then subclassed e.g. for the
IntegratorExecutor class, where a __getinitargs__ method is defined to
provide support for pickling. For Python <= 3.10 this works just fine.
However, bpo-26579 adds a default object.__getstate__() method in Python 3.11
which then triggers the 'Incomplete pickle support (__getstate_manages_dict__ not set)'
error message in Boost.Python. Adding '__getstate_manages_dict__ = 1'
to these subclasses appears to workaround the problem.
@rjgildea rjgildea requested a review from ndevenish May 31, 2023 16:10
rjgildea added a commit to regro-cf-autotick-bot/dials-feedstock that referenced this pull request May 31, 2023
rjgildea added a commit to conda-forge/dials-feedstock that referenced this pull request May 31, 2023
* Rebuild for python311

* MNT: Re-rendered with conda-build 3.24.0, conda-smithy 3.23.1, and conda-forge-pinning 2023.05.17.15.05.36

* Workaround Python 3.11 pickling bug

See dials/dials#2426

* Fix selector?

* Perhaps this is preferred?

---------

Co-authored-by: Richard Gildea <richard.gildea@diamond.ac.uk>
@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #2426 (c8200c9) into main (9cbc1e7) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head c8200c9 differs from pull request most recent head 818e5db. Consider uploading reports for the commit 818e5db to get more accurate results

@@            Coverage Diff             @@
##             main    #2426      +/-   ##
==========================================
- Coverage   78.61%   78.59%   -0.03%     
==========================================
  Files         603      604       +1     
  Lines       73927    73949      +22     
  Branches    10070    10070              
==========================================
+ Hits        58116    58118       +2     
- Misses      13671    13690      +19     
- Partials     2140     2141       +1     

@ndevenish ndevenish merged commit 5a0c928 into main Jun 12, 2023
16 checks passed
@ndevenish ndevenish deleted the getstate-pickle branch June 12, 2023 09:20
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.

None yet

3 participants