Skip to content

moved DummyMod to proper namespace to enable dill pickling#4186

Merged
takluyver merged 4 commits intoipython:masterfrom
mmckerns:master
Sep 11, 2013
Merged

moved DummyMod to proper namespace to enable dill pickling#4186
takluyver merged 4 commits intoipython:masterfrom
mmckerns:master

Conversation

@mmckerns
Copy link
Contributor

@mmckerns mmckerns commented Sep 9, 2013

enables pickling with dill, which worked in 0.13, but seemed to be broken in 1.0.

Issues remain, however, since dill.dump_session now saves "DummyMod" stub and not the user's interactive session. Needs more investigation into reason behind DummyMod, and if it's really a necessary evil.

@takluyver
Copy link
Member

IIRC, the reason for DummyMod is because we can't assign the __dict__ of a module directly. If user_ns and user_module aren't both passed in, we want to ensure we have only one namespace. If user_module.__dict__ is not user_ns, then things quickly get more complicated - I think this was giving us issues with pickling, in fact.

  • If user_module is passed in, we use its __dict__ as user_ns.
  • If neither is passed in, we create a new instance of ModuleType, and use its __dict__ as user_ns.
  • If user_ns is passed in, we create a DummyMod and assign user_ns to its __dict__.

There's more discussion of the rationale in PRs #384 and #648.

Copy link
Member

Choose a reason for hiding this comment

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

We have dropped support for Python 2.6 in master, so please don't commit this change. IPython 2.0 will support Python 2.7 and 3.3+.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That snuck in on me in the above pull request... sorry. I won't commit it.

@mmckerns
Copy link
Contributor Author

mmckerns commented Sep 9, 2013

@takluyver: popped out the 2.6 compatibility changes as requested. PR #4186 is now just moving DummyMod to the module's namespace. Sorry was a bit thrown off since README.rst still states 2.6 compatible. I do remember vaguely the "drop 2.6" discussion from the dev mailing list.

@takluyver
Copy link
Member

👍 Thanks Mike. We'll get the README updated.

@takluyver
Copy link
Member

Thanks @mmckerns . I think this looks sound, but I'll take a few minutes to test it after lunch.

Copy link
Member

Choose a reason for hiding this comment

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

Could you expand the docstring a little, now that it's no longer in context? Something like "...for IPython's interactive module when a namespace must be assigned to the modules __dict__".

Also, stick the @undoc decorator on the class so that it doesn't appear in the autogenerated docs.

@takluyver
Copy link
Member

This works fine in my manual tests - with those couple of small doc changes, I'll merge it.

@mmckerns
Copy link
Contributor Author

updated as requested. nice working with you on this.

@takluyver
Copy link
Member

Merging :-)

takluyver added a commit that referenced this pull request Sep 11, 2013
moved DummyMod to proper namespace to enable dill pickling
@takluyver takluyver merged commit a96f9c4 into ipython:master Sep 11, 2013
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
moved DummyMod to proper namespace to enable dill pickling
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.

2 participants