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

Move session-related code to new session module #530

Merged
merged 4 commits into from
Jul 23, 2022

Conversation

leogama
Copy link
Contributor

@leogama leogama commented Jul 22, 2022

Edit: updated.

Here is the relevant segment of the diff between the old _dill.py and the new session.py:

--- old/_dill.py	2022-07-22 14:13:01.157703539 -0300
+++ dill/session.py	2022-07-22 14:09:18.306036272 -0300
@@ -1,339 +1,43 @@
-# -*- coding: utf-8 -*-
+#!/usr/bin/env python
 #
 # Author: Mike McKerns (mmckerns @caltech and @uqfoundation)
+# Author: Leonardo Gama (@leogama)
 # Copyright (c) 2008-2015 California Institute of Technology.
 # Copyright (c) 2016-2022 The Uncertainty Quantification Foundation.
 # License: 3-clause BSD.  The full license text is available at:
 #  - https://github.com/uqfoundation/dill/blob/master/LICENSE
 """
-dill: a utility for serialization of python objects
-
-Based on code written by Oren Tirosh and Armin Ronacher.
-Extended to a (near) full set of the builtin types (in types module),
-and coded to the pickle interface, by <mmckerns@caltech.edu>.
-Initial port to python3 by Jonathan Dobson, continued by mmckerns.
-Test against "all" python types (Std. Lib. CH 1-15 @ 2.7) by mmckerns.
-Test against CH16+ Std. Lib. ... TBD.
+Pickle and restore the intepreter session.
 """
+
 __all__ = [
-    'dump', 'dumps', 'load', 'loads', 'dump_module', 'load_module',
-    'load_module_asdict', 'dump_session', 'load_session', 'Pickler', 'Unpickler',
-    'register', 'copy', 'pickle', 'pickles', 'check', 'HIGHEST_PROTOCOL',
-    'DEFAULT_PROTOCOL', 'PicklingError', 'UnpicklingError', 'HANDLE_FMODE',
-    'CONTENTS_FMODE', 'FILE_FMODE', 'PickleError', 'PickleWarning',
-    'PicklingWarning', 'UnpicklingWarning'
+    'dump_module', 'load_module', 'load_module_asdict',
+    'dump_session', 'load_session' # backward compatibility
 ]
 
-__module__ = 'dill'
-
-import warnings
-from .logger import adapter as logger
-from .logger import trace as _trace
-
-from typing import Optional, Union
-
-import os
+import re
 import sys
-----------------------------------<omitted>------------------------------------
+import warnings
+ 
+from dill import _dill, Pickler, Unpickler
+from ._dill import (
+    BuiltinMethodType, FunctionType, MethodType, ModuleType, TypeType,
+    _import_module, _is_builtin_module, _is_imported_module, _main_module,
+    _reverse_typemap, __builtin__,
+)
 
+# Type hints.
+from typing import Optional, Union
 
-### Pickle the Interpreter Session
 import pathlib
-import re
 import tempfile
-from types import SimpleNamespace
 
 TEMPDIR = pathlib.PurePath(tempfile.gettempdir())
 
 def _module_map():
     """get map of imported modules"""
     from collections import defaultdict
+    from types import SimpleNamespace
     modmap = SimpleNamespace(
         by_name=defaultdict(list),
         by_id=defaultdict(list),
@@ -349,10 +53,12 @@
             modmap.by_id[id(modobj)].append((modobj, objname, modname))
     return modmap
 
-SESSION_IMPORTED_AS_TYPES = (ModuleType, TypeType, FunctionType, MethodType, BuiltinMethodType)
-SESSION_IMPORTED_AS_MODULES = ('ctypes', 'typing', 'subprocess', 'threading',
+IMPORTED_AS_TYPES = (ModuleType, TypeType, FunctionType, MethodType, BuiltinMethodType)
+if 'PyCapsuleType' in _reverse_typemap:
+    IMPORTED_AS_TYPES += (_reverse_typemap['PyCapsuleType'],)
+IMPORTED_AS_MODULES = ('ctypes', 'typing', 'subprocess', 'threading',
                                r'concurrent\.futures(\.\w+)?', r'multiprocessing(\.\w+)?')
-SESSION_IMPORTED_AS_MODULES = tuple(re.compile(x) for x in SESSION_IMPORTED_AS_MODULES)
+IMPORTED_AS_MODULES = tuple(re.compile(x) for x in IMPORTED_AS_MODULES)
 
 def _lookup_module(modmap, name, obj, main_module):
     """lookup name or id of obj if module is imported"""
@@ -360,8 +66,8 @@
         if modobj is obj and sys.modules[modname] is not main_module:
             return modname, name
     __module__ = getattr(obj, '__module__', None)
-    if isinstance(obj, SESSION_IMPORTED_AS_TYPES) or (__module__ is not None
-            and any(regex.fullmatch(__module__) for regex in SESSION_IMPORTED_AS_MODULES)):
+    if isinstance(obj, IMPORTED_AS_TYPES) or (__module__ is not None
+            and any(regex.fullmatch(__module__) for regex in IMPORTED_AS_MODULES)):
         for modobj, objname, modname in modmap.by_id[id(obj)]:
             if sys.modules[modname] is not main_module:
                 return modname, objname
@@ -838,6 +544,7 @@
         >>> new_var in main # would be True if the option 'update' was set
         False
     """
     if 'module' in kwds:
         raise TypeError("'module' is an invalid keyword argument for load_module_asdict()")
     if hasattr(filename, 'read'):
@@ -869,1729 +576,3 @@
             pass
     main.__session__ = str(filename)
     return main.__dict__
-
-### End: Pickle the Interpreter
-----------------------------------<omitted>------------------------------------

@mmckerns
Copy link
Member

mmckerns commented Jul 22, 2022

I have three thoughts, even before reviewing: (1) you should make sure there are backward compatible imports for anything that you've moved to a new module, (2) would it be better to use main.__builtins__ = __builtins__ instead of importing builtins? (although it's probably a negligible difference), (3) the new module would need to be added to docs/source/dill.rst.

@leogama
Copy link
Contributor Author

leogama commented Jul 22, 2022

(1) you should make sure there are backward compatible imports for anything that you've moved to a new module

I was thinking about this just now.

(2) would it be better to use main.__builtins__ = __builtins__ instead of importing builtins? (although it's probably a negligible difference)

May be. I'll change this.

(3) the new module would need to be added to docs/source/dill.rst.

Already did that.

@leogama
Copy link
Contributor Author

leogama commented Jul 22, 2022

(1) you should make sure there are backward compatible imports for anything that you've moved to a new module

I was thinking about this just now.

There's a problem with this though. Adding from .session import * at the top of dill._dill will create circular import problems. I'll probably have to add it at the bottom. Actually, I had to export to dill._dill at the end of dill.session.

Should I include 'dump_session' and 'load_session' in dill._dill.__all__?

@leogama
Copy link
Contributor Author

leogama commented Jul 22, 2022

@mmckerns: applied the requested changes.

Copy link
Member

@mmckerns mmckerns left a comment

Choose a reason for hiding this comment

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

LGTM

@mmckerns mmckerns merged commit 5a66152 into uqfoundation:master Jul 23, 2022
@leogama leogama deleted the session-submodule branch July 23, 2022 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants