Can't Load Russian Language Model due to Caching Bug

[jonwiggins]

steps to reproduce

>>> import textacy
>>> nlp = textacy.load_spacy_lang("ru_core_news_lg")

yields

... python3.7/site-packages/textacy/cache.py", line 38, in _get_size
    size += sum((_get_size(i, seen) for i in obj))
TypeError: 'type' object is not iterable

expected vs. actual behavior

The Russian Language model should be loaded

possible solution?

This is because the library which the model uses for Russian and Ukrainian morphology (pymorphy2: https://github.com/kmike/pymorphy2) has a class that exhibits some odd behavior.

(Pdb) obj
<class 'pymorphy2.analyzer.Parse'>
(Pdb) type(obj)
<class 'type'>
(Pdb) hasattr(obj, "__iter__")
True

The caching logic (https://github.com/chartbeat-labs/textacy/blob/main/src/textacy/cache.py)

 elif hasattr(obj, "__iter__") and not isinstance(obj, (str, bytes, bytearray)):
        size += sum((_get_size(i, seen) for i in obj))

Thinks that obj is iterable, and then fails.

A similar bug has been reported with OrderedDict on the original repo for this caching code (bosswissam/pysize#8) but has not been fixed.

I think that there are three possible fixes.

1. Change the check for this to be ... and not isinstance(obj, (str, bytes, bytearray, type)). This will fix this specific instance of the bug, but will not fix any possible occurrence of a strange object which has __iter__ but cannot be iterated.
2. Wrap this operation in a try/except, this will probably have some performance hit.
3. Change the first part of the condition to isinstance(obj, collections.Iterable).

The python documentation (https://docs.python.org/3/library/collections.abc.html#collections.abc.Iterable) actually mentions that the third option will not work in all cases, and seems to recommend using a try/except to see if the object can be iterated over. Although I feel like that is a less elegant solution.

@bdewilde If you have feelings towards one of these solutions I'm happy to open a PR to make the change.

environment

• operating system: MacOS
• python version: 3.7.5
• spacy version: 3.2.1
• installed spacy models: Several
• textacy version: 0.12

[bdewilde]

hi @jonwiggins ! thank you tons for finding and digging into this bug. it looks like pymorphy2.analyzer.Parse inherits from collections.namedtuple, and that has the unexpected __iter__/Iterable behavior. i checked, and i think your option 3 stays closest to the original code and should do the trick in this case:

>>> import collections
>>> import pymorphy2
>>> isinstance(pymorphy2.analyzer.Parse, collections.abc.Iterable)
False
>>> hasattr(pymorphy2.analyzer.Parse, "__iter__")
True

i would be delighted to accept your PR with the change. i don't have any immediate plans to cut a new release, but if this is a blocker on cb work, i could probably do a small bugfix release... let me know 👍

[jonwiggins]

Sounds good. FWIW, I merged option 2 into the pysize repo: bosswissam/pysize#18

Upon review option 3 isn't as great because it is possible for iterable objects to not have collections.abc.Iterable as their parent class. https://stackoverflow.com/questions/32799980/what-exactly-does-iterable-mean-in-python-why-isnt-my-object-which-implement

It's not a huge blocker but it is something I'd like to get fixed.

[bdewilde]

@jonwiggins ah, nice! let's follow the source reference's (your) example and wrap the call in a try/except, with one minor difference: set the logging message's level to WARNING, since the implications for textacy's usage aren't too serious. thanks in advance!

[bdewilde]

fixed in main, closing!