enthought / enthought/apptools
Usage of updater in VersionedUnpickler is undocumented and maybe redundant
- Dominant language
- Python
- Stars
- 38
- Forks
- 21
- PR merge metrics
- No merged PRs in 30d
Description
The `VersionedUnpickler` accepts an updater object:
https://github.com/enthought/apptools/blob/632a4bfd7ce861b985a3d9dd8c25e44e5b433f0f/apptools/persistence/versioned_unpickler.py#L111-L113
This was supposed to be its "interface":
https://github.com/enthought/apptools/blob/632a4bfd7ce861b985a3d9dd8c25e44e5b433f0f/apptools/persistence/updater.py#L9-L17
But from where it is used, there are more features expected of this interface, e.g. it is expected to have an attribute called `setstates` which is a dictionary:
https://github.com/enthought/apptools/blob/632a4bfd7ce861b985a3d9dd8c25e44e5b433f0f/apptools/persistence/versioned_unpickler.py#L157
The structure of this dictionary is not documented (and there seem to be no tests for it). It seems to be used for monkeypatching a class `__setstate__` (a global state that does not belong to apptools!), but it may not be restored.
Monkeypatching here in the code path if `VersionedUnpickler.updater` is not None:
https://github.com/enthought/apptools/blob/632a4bfd7ce861b985a3d9dd8c25e44e5b433f0f/apptools/persistence/versioned_unpickler.py#L166-L167
To be restored if some other unpickler without an updater came across the same class:
https://github.com/enthought/apptools/blob/632a4bfd7ce861b985a3d9dd8c25e44e5b433f0f/apptools/persistence/versioned_unpickler.py#L145-L148
It is possible that no one uses an updater with the `VersionedUnpickler` so this monkeypatching is never exercised. I have not checked if there are other uses of an updater anywhere else. It could be a feature that can be removed.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.