Pickle inner collections.namedtuples and function attributes

[anivegesana]

1. Fixing pickling of collections.namedtuple subclasses
2. Fixes pickling collections.namedtuple when __qualname__ is not __name__
3. Saves nonessential attributes of function objects by name instead arguments to the _create_function (supersedes Pickle function attributes #422)
4. Pickles defaults of collections.namedtuple classes

I think it is worth debating if this is the best way to handle new attributes to the function class. Some functions have __annotations__ but not __kwdefaults__. In the future, if these new attributes don't scale well, the current solution of adding them to _create_function could likely lead to a mess. On the other hand, these new strings would be added to all pickles whose functions that use said attributes. When many functions are pickled in one pickle, this overhead is constant. However, when pickling many functions in separate pickles, the overhead becomes linear in the number of functions. This cost should be considered.

This PR also adds a breaking change. Previously, _create_namedtuple did not save and use the default values of the tuple. In order to add the argument to this function to support it, pickling a collections.namedtuple with default values using this PR and trying to unpickle it in an older version of dill will fail.

Fixes #288, Fixes #420

[anivegesana]

I believe that the only issue for this PR is coverage. Some cases apply to Python 3.6 where defaults are not available for collections.namedtuple and some for Python 3.7+ when this feature was added.

[mmckerns]

Whenever you are ready for a review (on this or other of your PRs), just request me as a reviewer.

[anivegesana]

I think that requires permissions, just like merging an already approved PR. The open PRs are ready to review. I am still refactoring the draft and adding test cases, but it is functional.

[anivegesana]

Looking back on it, I think that this PR might be a little incomplete. I think that I need to ensure that every slot defined under "User-defined functions" on https://docs.python.org/3/reference/datamodel.html gets copied over when it isn't already the default value.

[anivegesana]

This PR seems to be backwards incompatible for PyPy 2.7. A function pickled in this PR will be unable to be unpickled in dill 0.3.4 on PyPy 2.7, but will be able to be unpickled on all other versions. Should there be a setting to turn off this feature to allow for the creation of backwards compatible pickles?

[mmckerns]

This PR seems to be backwards incompatible for PyPy 2.7. A function pickled in this PR will be unable to be unpickled in dill 0.3.4 on PyPy 2.7, but will be able to be unpickled on all other versions. Should there be a setting to turn off this feature to allow for the creation of backwards compatible pickles?

Ouch. No. The good thing is that it's only for PyPy 2.7. I'll have to have a look and see if there's any way to avoid the incompatibility.

[anivegesana]

There is an easy way. Instead of using __setstate__, I unroll it into a chain of setattr calls. It is just more inefficient. That is why a setting for this might be useful.

This actually seems like an incredibly viable solution. After looking into it a bit more, the only two attributes for functions that are not available in the constructor for Python 2.7 are __doc__ and __module__. "Unrolling it" for Python 2 always makes perfect sense, so a new setting isn't needed. There would only be one very quick additional operation only for functions with doc strings, so the overhead is minimal.

__dict__ should also be changed after the creation of the function to allow for code similar to the mystic example to work, but this will add the constant overhead of having an additional setattr call for every function when unpickling in Python 2.7. Do you think that this is worth it or should we just drop this functionality for Python 2.7?

[mmckerns]

What's done with python 2.7 doesn't matter too much. I will be dropping support for 2.7 immediately after the next release. With regard to what to do, I'd rather not lose functionality from one release to the next -- and would much prefer living with a small hit on the time it takes to pickle something in 2.7. So, yes, the overhead of a setattr call is preferred.

The basic rule is don't remove functionality.

[anivegesana]

Got it.

[mmckerns]

There's an issue if the dill pickler is not used. I typically check is_dill to guard against missing attributes.

$ python test_readwrite.py 
Traceback (most recent call last):
  File "./test_readwrite.py", line 144, in <module>
    test_archive()
  File "./test_readwrite.py", line 122, in test_archive
    check_basic(archive)
  File "./test_readwrite.py", line 80, in check_basic
    d['d'] = squared
  File "/Users/mmckerns/lib/python3.7/site-packages/klepto-0.2.2.dev0-py3.7.egg/klepto/_archives.py", line 397, in __setitem__
    self._store(key, value, input=False) # input=True also stores input
  File "/Users/mmckerns/lib/python3.7/site-packages/klepto-0.2.2.dev0-py3.7.egg/klepto/_archives.py", line 621, in _store
    protocol=protocol)
  File "/Users/mmckerns/lib/python3.7/site-packages/klepto-0.2.2.dev0-py3.7.egg/klepto/_pickle.py", line 389, in dump
    pickler.dump(value)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/pickle.py", line 437, in dump
    self.save(obj)
  File "/Users/mmckerns/lib/python3.7/site-packages/klepto-0.2.2.dev0-py3.7.egg/klepto/_pickle.py", line 264, in save
    return Pickler.save(self, obj)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/pickle.py", line 504, in save
    f(self, obj) # Call unbound method with explicit self
  File "/Users/mmckerns/lib/python3.7/site-packages/dill-0.3.5.dev0-py3.7.egg/dill/_dill.py", line 1921, in save_function
    topmost_postproc = next(iter(pickler._postproc.values()), None)
AttributeError: 'NumpyPickler' object has no attribute '_postproc'

[anivegesana]

Good catch. I guess I forgot to copy that code from the merge commit.

[mmckerns]

LGTM