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 pickling when pytest is loaded. #107

Merged
merged 1 commit into from
Jul 26, 2017
Merged

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Jul 10, 2017

Older versions of pytest will add a "None" module to sys.modules.

This was removed rather recently pytest-dev/pytest#2106, so this PR prevents that old code from breaking things.

@codecov-io
Copy link

codecov-io commented Jul 10, 2017

Codecov Report

Merging #107 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #107   +/-   ##
=======================================
  Coverage   83.12%   83.12%           
=======================================
  Files           2        2           
  Lines         551      551           
  Branches      107      107           
=======================================
  Hits          458      458           
  Misses         65       65           
  Partials       28       28
Impacted Files Coverage Δ
cloudpickle/cloudpickle.py 83.02% <100%> (ø) ⬆️

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 eb49847...f2269de. Read the comment docs.

@@ -384,7 +384,7 @@ def _save_subimports(self, code, top_level_dependencies):
# check if the package has any currently loaded sub-imports
prefix = x.__name__ + '.'
for name, module in sys.modules.items():
if name.startswith(prefix):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to have a comment here about why we check on the name being None.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

Older versions of pytest will add a "None" module to sys.modules.
@rgbkrk
Copy link
Member

rgbkrk commented Jul 26, 2017

Thanks!

@rgbkrk rgbkrk merged commit e6de4f4 into cloudpipe:master Jul 26, 2017
@QuLogic QuLogic deleted the old-pytest branch July 26, 2017 00:39
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.

3 participants