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

New auxiliary function to get value for options: _getopt() #555

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions dill/_dill.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,9 @@ def dump(obj, file, protocol=None, byref=None, fmode=None, recurse=None, **kwds)
See :func:`dumps` for keyword arguments.
"""
from .settings import settings
protocol = settings['protocol'] if protocol is None else int(protocol)
_kwds = kwds.copy()
_kwds.update(dict(byref=byref, fmode=fmode, recurse=recurse))
Pickler(file, protocol, **_kwds).dump(obj)
protocol = int(_getopt(settings, 'protocol', protocol))
kwds.update(byref=byref, fmode=fmode, recurse=recurse)
Pickler(file, protocol, **kwds).dump(obj)
return

def dumps(obj, protocol=None, byref=None, fmode=None, recurse=None, **kwds):#, strictio=None):
Expand Down Expand Up @@ -317,6 +316,26 @@ class PicklingWarning(PickleWarning, PicklingError):
class UnpicklingWarning(PickleWarning, UnpicklingError):
pass

def _getopt(settings, key, arg=None, *, kwds=None):
"""Get option from named argument 'arg' or 'kwds', falling back to settings.

Examples:

# With an explicitly named argument:
protocol = int(_getopt(settings, 'protocol', protocol))

# With a named argument in **kwds:
self._byref = _getopt(settings, 'byref', kwds=kwds)
"""
# Sanity check, it's a bug in calling code if False.
assert kwds is None or arg is None
Copy link
Member

Choose a reason for hiding this comment

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

To me, an AssertionError is not expected... shouldn't it be a ValueError?

if kwds is not None:
arg = kwds.pop(key, None)
if arg is not None:
return arg
else:
return settings[key]
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit unorthodox that the alternate is given as the first two values, and then the arg/kwd you are primarily interested in is given after that. Consider the syntax of getattr and similar.

So, this function pops key out of kwds (isn't get a better choice?), and if key is found and not None, then it is returned. If not found, then use arg if arg is provided, unless it's None... then default back to settings[key]. It's also expected that arg and kwds are never both given... so essentially, you have two one-liners in a single function, where any interaction between the two one-liners is an error. I'm not seeing the utility of this function, in that case. I could see using it, maybe, if you wanted to hide the import of settings within the function -- so _getopt(key, arg=None, *, kwds=None).


### Extend the Picklers
class Pickler(StockPickler):
"""python's Pickler extended to interpreter sessions"""
Expand All @@ -326,19 +345,15 @@ class Pickler(StockPickler):

def __init__(self, file, *args, **kwds):
settings = Pickler.settings
_byref = kwds.pop('byref', None)
#_strictio = kwds.pop('strictio', None)
_fmode = kwds.pop('fmode', None)
_recurse = kwds.pop('recurse', None)
StockPickler.__init__(self, file, *args, **kwds)
self._main = _main_module
self._diff_cache = {}
self._byref = settings['byref'] if _byref is None else _byref
self._strictio = False #_strictio
self._fmode = settings['fmode'] if _fmode is None else _fmode
self._recurse = settings['recurse'] if _recurse is None else _recurse
self._byref = _getopt(settings, 'byref', kwds=kwds)
self._fmode = _getopt(settings, 'fmode', kwds=kwds)
self._recurse = _getopt(settings, 'recurse', kwds=kwds)
self._strictio = False #_getopt(settings, 'strictio', kwds=kwds)
self._postproc = OrderedDict()
self._file = file
StockPickler.__init__(self, file, *args, **kwds)

def save(self, obj, save_persistent_id=True):
# register if the object is a numpy ufunc
Expand Down Expand Up @@ -410,10 +425,9 @@ def find_class(self, module, name):

def __init__(self, *args, **kwds):
settings = Pickler.settings
_ignore = kwds.pop('ignore', None)
StockUnpickler.__init__(self, *args, **kwds)
self._main = _main_module
self._ignore = settings['ignore'] if _ignore is None else _ignore
self._ignore = _getopt(settings, 'ignore', kwds=kwds)
StockUnpickler.__init__(self, *args, **kwds)

def load(self): #NOTE: if settings change, need to update attributes
obj = StockUnpickler.load(self)
Expand Down
16 changes: 12 additions & 4 deletions dill/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
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__,
_getopt, _import_module, _is_builtin_module, _is_imported_module,
_main_module, _reverse_typemap, __builtin__,
)

# Type hints.
Expand Down Expand Up @@ -211,8 +211,10 @@ def dump_module(
refimported = kwds.pop('byref', refimported)
module = kwds.pop('main', module)

from .settings import settings
protocol = settings['protocol']
from .settings import settings as dill_settings
protocol = dill_settings['protocol']
refimported = _getopt(settings, 'refimported', refimported)

main = module
if main is None:
main = _main_module
Expand Down Expand Up @@ -576,6 +578,12 @@ def load_module_asdict(
main.__session__ = str(filename)
return main.__dict__

## Session settings ##

settings = {
'refimported': False,
}


# Internal exports for backward compatibility with dill v0.3.5.1
# Can't be placed in dill._dill because of circular import problems.
Expand Down