Skip to content

Commit

Permalink
Fixed AttribiteError when unpickling; Optimized pickling
Browse files Browse the repository at this point in the history
Details:

* When unpickling an object of NocaseList, an AttributeError is raised
  in extend(), stating that _lc_list is missing. Adding debug prints
  reveals that pickle.load() calls extend() on a NocaseList object
  that has not been initialized with __init__().

  This change mitigates that by handling the exception in extend()
  and setting _lc_list freshly.

* This change also adds __setstate__() and __getstate__() methods
  that optimize the serialized data by only having the original
  list serialized. For unknown reasons, pickle does not call
  __setstate__() on Python 2 for the NocaseList class. Therefore,
  the __getstate__() and __setstate__() methods are deleted when
  running on Python 2, unless the documentation is built.

Signed-off-by: Andreas Maier <andreas.r.maier@gmx.de>
  • Loading branch information
andy-maier committed Sep 11, 2020
1 parent 581b3af commit be8d83a
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 2 deletions.
8 changes: 8 additions & 0 deletions docs/changes.rst
Expand Up @@ -16,8 +16,16 @@ Released: not yet

**Bug fixes:**

* Fixed an AttributeError during unpickling. (See issue #37)

**Enhancements:**

* Optimized pickling a NocaseList object by serializing only the original
list, but not the second lower-cased list. This optimization is only
implemented for Python 3.

* Added tests for pickling and unpickling.

**Cleanup:**

* Suppressed new Pylint issue 'super-with-arguments', because this package
Expand Down
5 changes: 5 additions & 0 deletions docs/conf.py
Expand Up @@ -91,6 +91,11 @@ def get_version(version_file):
else:
master_doc = 'docs/index'

# This env var is evaluated in the nocaselist package and causes the methods
# that are supposed to exist only in a particular Python version, not to be
# removed, so they appear in the docs.
os.environ['BUILDING_DOCS'] = '1'

# General information about the project.
project = u'nocaselist'
#copyright = u''
Expand Down
54 changes: 52 additions & 2 deletions nocaselist/_nocaselist.py
Expand Up @@ -5,6 +5,7 @@
from __future__ import print_function, absolute_import

import sys
import os

__all__ = ['NocaseList']

Expand All @@ -14,6 +15,11 @@
else:
_INTEGER_TYPES = (int,)

# This env var is set when building the docs. It causes the methods
# that are supposed to exist only in a particular Python version, not to be
# removed, so they appear in the docs.
BUILDING_DOCS = os.environ.get('BUILDING_DOCS', False)


def _lc_list(lst):
"""
Expand Down Expand Up @@ -47,6 +53,9 @@ class NocaseList(list):
The implementation maintains a second list with the lower-cased items of
the inherited list, and ensures that both lists are in sync.
The list supports serialization via the Python :mod:`py:pickle` module.
To save space and time, only the originally cased list is serialized.
""" # noqa E401
# pylint: enable=line-too-long

Expand Down Expand Up @@ -88,6 +97,33 @@ def __init__(self, iterable=()):
else:
self._lc_list = _lc_list(self)

def __getstate__(self):
"""
Called when pickling the object, see :meth:`py:object.__getstate__`.
In order to save space and time, only the list with the originally
cased items is saved, but not the second list with the lower cased
items.
On Python 2, the 'pickle' module does not call :meth:`__setstate__`,
so this optimzation has only be implemented for Python 3.
"""
# This copies the state of the inherited list even though it is
# not visible in self.__dict__.
state = self.__dict__.copy()
del state['_lc_list']
return state

def __setstate__(self, state):
"""
Called when unpickling the object, see :meth:`py:object.__setstate__`.
On Python 2, the 'pickle' module does not call this method, so this
optimzation has only be implemented for Python 3.
"""
self.__dict__.update(state)
self._lc_list = _lc_list(self)

def __setitem__(self, index, value):
"""
Update the value of the item at an existing index in the list.
Expand Down Expand Up @@ -422,8 +458,14 @@ def extend(self, iterable):
method.
"""
super(NocaseList, self).extend(iterable)
for value in iterable:
self._lc_list.append(value.lower())
# The following is a circumvention for a behavior of the 'pickle' module
# that during unpickling may call this method on an object that has
# been created with __new__() without calling __init__().
try:
for value in iterable:
self._lc_list.append(value.lower())
except AttributeError:
self._lc_list = _lc_list(self)

def insert(self, index, value):
"""
Expand Down Expand Up @@ -489,3 +531,11 @@ def lower_key(value):

super(NocaseList, self).sort(key=lower_key, reverse=reverse)
self._lc_list = _lc_list(self)


# Remove methods that should only be present in a particular Python version.
# If the documentation is being built, the methods are not removed in order to
# show them in the documentation.
if sys.version_info[0] == 2 and not BUILDING_DOCS:
del NocaseList.__setstate__
del NocaseList.__getstate__
57 changes: 57 additions & 0 deletions tests/unittest/test_nocaselist.py
Expand Up @@ -7,6 +7,7 @@
import sys
import os
import re
import pickle
import pytest

from ..utils.simplified_test_function import simplified_test_function
Expand Down Expand Up @@ -2697,3 +2698,59 @@ def test_NocaseList_sort(testcase, nclist, kwargs, exp_nclist):

assert result is None
assert_equal(nclist_copy, exp_nclist)


TESTCASES_NOCASELIST_PICKLE = [

# Testcases for pickling and unpickling NocaseList objects

# Each list item is a testcase tuple with these items:
# * desc: Short testcase description.
# * kwargs: Keyword arguments for the test function:
# * nclist: NocaseList object to be used for the test.
# * exp_exc_types: Expected exception type(s), or None.
# * exp_warn_types: Expected warning type(s), or None.
# * condition: Boolean condition for testcase to run, or 'pdb' for debugger

(
"Empty list",
dict(
nclist=NocaseList(),
),
None, None, True
),
(
"List with two items",
dict(
nclist=NocaseList(['Dog', 'cat']),
),
None, None, True
),
]


@pytest.mark.parametrize(
"desc, kwargs, exp_exc_types, exp_warn_types, condition",
TESTCASES_NOCASELIST_PICKLE)
@simplified_test_function
def test_NocaseList_pickle(testcase, nclist):
"""
Test function for pickling and unpickling NocaseList objects
"""

# Don't change the testcase data, but a copy
nclist_copy = NocaseList(nclist)

# Pickle the object
pkl = pickle.dumps(nclist_copy)

del nclist_copy

# Unpickle the object
nclist2 = pickle.loads(pkl)

# Ensure that exceptions raised in the remainder of this function
# are not mistaken as expected exceptions
assert testcase.exp_exc_types is None

assert_equal(nclist2, nclist)

0 comments on commit be8d83a

Please sign in to comment.