some errors with dump_module and load_module #525
Comments
|
@leogama: I also noticed that it might be more natural to replace the argument |
I was about to propose this yesterday... Do you think it suffices to change the parameter name, or would you change the local variable names too? I'll take a look at the errors you pointed to. |
|
Changing the name of the argument is probably the easiest route. Then you can use |
|
O crap... I think I added the statement |
The behavior is exactly the expected, as
The error message can be made more explicit however. Instead of this and variations:
It could be:
This behavior is consistent with |
…qfoundation#525) New phrasing of mismatching modules error messages in load_session(): ```python >>> import dill >>> dill.dump_module() >>> dill.load_module(module='math') ValueError: can't update module 'math' with the saved state of module '__main__' >>> import types >>> main = types.ModuleType('__main__') >>> dill.load_module(module=main) ValueError: can't update module-type object '__main__' with the saved state of imported module '__main__' >>> dill.dump_module(module=main) >>> dill.load_module(module='__main__') ValueError: can't update imported module '__main__' with the saved state of module-type object '__main__' ```
That's good. I wanted to confirm this was the intended behavior. We need to be very clear in the docs what will happen, and the errors should also be clear so people who ignore the docs should know why the error was thrown. |
* fix dump_module() bugs and rename parameter 'main' to 'module' (fixes #525) New phrasing of mismatching modules error messages in load_session(): ```python >>> import dill >>> dill.dump_module() >>> dill.load_module(module='math') ValueError: can't update module 'math' with the saved state of module '__main__' >>> import types >>> main = types.ModuleType('__main__') >>> dill.load_module(module=main) ValueError: can't update module-type object '__main__' with the saved state of imported module '__main__' >>> dill.dump_module(module=main) >>> dill.load_module(module='__main__') ValueError: can't update imported module '__main__' with the saved state of module-type object '__main__' ``` * dump_module: clarify refimport description * improvements to 'refimported' handling and extra checks in *_module() functions * load_session(): clarify that the 'module' argument must match the session file's module
|
@leogama: Can you address the following?
|
Sure. This is my current understanding: First, the premise of Pragmatically, we go back to that discussion in #475 about a filter for unpickleable objects, which could be used to allow, for example, any built-in module to be saved seamlessly. I didn't answer about that until now because I was pondering what kind of implementation would be more reasonable for this particular problem, which (as we see here) is broader in scope than simple objects to save selection. I'm reaching the conclusion that it's better to have a dedicated option for it in I just didn't get the last part of your question. What do you mean by pickling a built-in module by reference? The whole module, like
I added a Again, didn't get the last bit. What would throw a is_runtime_mod = pickle_main.startswith('__runtime__.')
It's correct. For importable/imported modules, returning the module is just a convenience for not having to explicitly import the restored module to be able to access it. What is restored is always the So these three sequences of statements are equivalent: >>> import dill
>>> import test # load module 'test' and assign it to name 'test'
>>> dill.load_module('test.pkl') # update it
<module 'test' from '/usr/lib/python3.8/test/__init__.py'>
>>> test.x
1
# in another session:
>>> import dill
>>> dill.load_module('test.pkl') # load module 'test' and update it
<module 'test' from '/usr/lib/python3.8/test/__init__.py'>
>>> import test # assign the name 'test' to the existent sys.modules['test'] object
>>> test.x
1
# in yet another session:
>>> import dill
>>> test = dill.load_module('test.pkl') # load, update and assign in a single statement
>>> test.x
1Note that, in the first case, if the parameter
What you did is not much different than doing: >>> import test
>>> test.x
AttributeError: module 'test' has no attribute 'x'
>>> test.x = 1
>>> del test
>>> import test as _test
>>> _test.x
1As the |
For modules, generically, So, there might be some expectations that Thus, we need to (A) make the conceptual distinction clear (as I did above) in the docs, and (B) think about what should happen when
What I meant is that, imagine you are given a
Right. So the three sequences you gave should be part of the documentation, so it's explicitly understood what is going on. One might assume that because you are assigning a name to a module that is returned from Do each of the above points make sense? |
|
Yes, pretty much everything you wrote makes sense. I think part of this extended documentation should go into the specific functions' docstrings and part to the package's general documentation. How can we write these with four hands? I could open a PR and you could suggest changes to my additions and write entirely new paragraphs using the "suggested changes" feature in the code review page. (If you don't find the "Add a suggestion" button, take a look here: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#adding-line-comments-to-a-pull-request) Edit: just opened the PR (#527) |
In trying out
dump_moduleandload_modulea bit more extensively after #507, and I ran across some interesting cases:mathalso fails to load when saved by name~ new session ~
dump_module(artifact from case 1 above).The text was updated successfully, but these errors were encountered: