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

Resolve dill incompatibility with attempt_import. #2419

Merged
merged 6 commits into from
May 24, 2022

Conversation

jsiirola
Copy link
Member

Fixes #2414

Summary/Motivation:

Current dill implementations look for the dill module by looking for the _dill attribute on all objects in the globals(). This was causing missing modules (ModuleUnavailable objects) to raise a DeferredImportError. This adds a special case to that this behavior does not trigger the DeferredImportError (and instead raises the "expected" AttributeError.

This also makes the ModuleUnavailable and DeferredImportModule classes picklable.

Changes proposed in this PR:

  • Do not trigger a DeferredImportError when attempting to retrieve a _dill attribute
  • Make ModuleUnavailable and DeferredImportModule classes picklable.

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

pyomo/common/dependencies.py Outdated Show resolved Hide resolved
@mrmundt mrmundt mentioned this pull request May 23, 2022
@codecov
Copy link

codecov bot commented May 23, 2022

Codecov Report

Merging #2419 (bedc0ef) into main (42ce167) will increase coverage by 1.48%.
The diff coverage is 50.00%.

❗ Current head bedc0ef differs from pull request most recent head 297f948. Consider uploading reports for the commit 297f948 to get more accurate results

@@            Coverage Diff             @@
##             main    #2419      +/-   ##
==========================================
+ Coverage   84.41%   85.89%   +1.48%     
==========================================
  Files         617      617              
  Lines       76183    76192       +9     
==========================================
+ Hits        64309    65448    +1139     
+ Misses      11874    10744    -1130     
Flag Coverage Δ
linux 82.63% <50.00%> (+22.93%) ⬆️
osx 72.86% <50.00%> (?)
other 82.82% <50.00%> (+23.13%) ⬆️
win 79.64% <50.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pyomo/common/dependencies.py 97.03% <50.00%> (-1.44%) ⬇️
pyomo/contrib/pynumero/sparse/block_matrix.py 89.96% <0.00%> (+0.16%) ⬆️
pyomo/solvers/plugins/solvers/gurobi_direct.py 71.91% <0.00%> (+0.16%) ⬆️
pyomo/core/expr/logical_expr.py 88.30% <0.00%> (+0.23%) ⬆️
pyomo/contrib/interior_point/interior_point.py 95.03% <0.00%> (+0.26%) ⬆️
pyomo/solvers/plugins/solvers/GUROBI.py 88.52% <0.00%> (+0.32%) ⬆️
pyomo/contrib/pynumero/sparse/block_vector.py 86.86% <0.00%> (+0.35%) ⬆️
pyomo/core/base/expression.py 90.04% <0.00%> (+0.49%) ⬆️
pyomo/core/base/units_container.py 98.08% <0.00%> (+0.54%) ⬆️
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42ce167...297f948. Read the comment docs.

@jsiirola jsiirola requested a review from mrmundt May 23, 2022 23:14
@mrmundt
Copy link
Contributor

mrmundt commented May 24, 2022

@jsiirola - I'll look into resolving the failing tests. I know you're busy today.

@jsiirola
Copy link
Member Author

@mrmundt, I was just working in it. I think I just pushed a valid change that should fix the pickle issue.

@mrmundt
Copy link
Contributor

mrmundt commented May 24, 2022

The failing tests are now all passing. I will merge the PR.

@mrmundt mrmundt merged commit fc02a1c into Pyomo:main May 24, 2022
@jsiirola jsiirola deleted the dill-compatibility branch May 24, 2022 17:44
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 failure in kernel with Dill 0.3.5.1
2 participants