tantale / deprecated

Python @deprecated decorator to deprecate old python classes, functions or methods.
MIT License
301 stars 32 forks source link

decorated methods with "deprecated" cannot be pickled #16

Closed garciparedes closed 4 years ago

garciparedes commented 4 years ago

Hi! First of all, thank you so much for writing this library, it really helps me maintaining deprecations around my source code.

Nevertheless, I realised that there is one problem related with the serialization process (with both pickle and dill) which I believe is related with the wrapt library and the https://github.com/GrahamDumpleton/wrapt/issues/102 issue, in which the author aims to implement the __reduce_ex__.

Expected Behavior

Tell us what should happen.

from pickle import dumps

from deprecated import deprecated

@deprecated
def foo():
    return 'bar'

print(dumps(foo))

Actual Behavior

Tell us what happens instead.

Traceback (most recent call last):
  File "~/development/open-source/deprecated/proof.py", line 11, in <module>
    print(dumps(foo))
NotImplementedError: object proxy must define __reduce_ex__()

In case of replacing the pickle module by dill, the error trace is more descriptive:

Traceback (most recent call last):
  File "/Applications/PyCharm.app/Contents/plugins/python/helpers/pydev/pydevd.py", line 1434, in _exec
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/Applications/PyCharm.app/Contents/plugins/python/helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/Users/garciparedes/development/open-source/deprecated/proof.py", line 11, in <module>
    print(dumps(foo))
  File "/Users/garciparedes/development/open-source/deprecated/venv/lib/python3.7/site-packages/dill/_dill.py", line 265, in dumps
    dump(obj, file, protocol, byref, fmode, recurse, **kwds)#, strictio)
  File "/Users/garciparedes/development/open-source/deprecated/venv/lib/python3.7/site-packages/dill/_dill.py", line 259, in dump
    Pickler(file, protocol, **_kwds).dump(obj)
  File "~/development/open-source/deprecated/venv/lib/python3.7/site-packages/dill/_dill.py", line 445, in dump
    StockPickler.dump(self, obj)
  File "/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/pickle.py", line 437, in dump
    self.save(obj)
  File "/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/pickle.py", line 524, in save
    rv = reduce(self.proto)
NotImplementedError: object proxy must define __reduce_ex__()

Environment

tantale commented 4 years ago

You’re welcome.

Never seen this issue before. Need time to investigate: I don’t really know where to define the __reduce_ex__() function (and why).

Hum! This must be Deprecated v1.2.7. The 1.2.8 is about to be released 😉

tantale commented 4 years ago

Well, fixing the bug you reported is quite complex.

The problem actually comes from the __reduce_ex__ function which is not implemented. According to @GrahamDumpleton: "Depending on the use case you may or may not want the original wrapped object to pickled".

It's clear that in a general context, we can't take a decision, but here, in the context of the @deprecated decorator we can assume the decorated function should be pickled.

The problem is how.

One of the person who chat in the issue tracker gives an example. We can start with this example to implement a __reduce_ex__ function which is able to serialize a decorated function…

tantale commented 4 years ago

It is very difficult to fix this bug, maybe impossible without diving deep inside Wrapt core library. Won't fix, sorry.