ryanjgallagher / shifterator

Interpretable data visualizations for understanding how texts differ at the word level
Apache License 2.0
273 stars 29 forks source link

Collections.Mapping is depreciated for python versions >3.10 #41

Open mvarnold opened 1 year ago

mvarnold commented 1 year ago

Getting some Attribute Errors when making shifts if using a newer version of python.

 File "/Users/michael/projects/data-mountain-query/data_mountain_query/sentiment_plot.py", line 378, in general_sentiment_shift
    sentiment_shift = sh.WeightedAvgShift(type2freq_1=type2freq_1,
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/michael/miniconda3/envs/data-mountain-query/lib/python3.11/site-packages/shifterator/shifts.py", line 59, in __init__
    super().__init__(
  File "/Users/michael/miniconda3/envs/data-mountain-query/lib/python3.11/site-packages/shifterator/shifterator.py", line 72, in __init__
    self.type2score_1, lex_ref = helper.get_score_dictionary(type2score_1)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/michael/miniconda3/envs/data-mountain-query/lib/python3.11/site-packages/shifterator/helper.py", line 156, in get_score_dictionary
    if isinstance(scores, collections.Mapping):
                          ^^^^^^^^^^^^^^^^^^^
AttributeError: module 'collections' has no attribute 'Mapping'

It's an easy fix, just need to change line 156 in helper.py from

    if isinstance(scores, collections.Mapping):

to

    if isinstance(scores, collections.abc.Mapping):

see this stack overflow post for more: https://stackoverflow.com/questions/69381312/in-vs-code-importerror-cannot-import-name-mapping-from-collections

mvarnold commented 1 year ago

Actually, if you want to make it backward compatible and not deal with different versions of shifterator for different versions of python, I would add something like this:

try:
    from collections.abc import Mapping
except ImportError:
    from collections import Mapping

in the imports and then just edit to have Mapping on line 156

if isinstance(scores, Mapping):

instead of the existing collections.Mapping

andyreagan commented 4 months ago

Closed by #40

mvarnold commented 3 months ago

@andyreagan I'm not sure this got pushed to the pypi version because some people are still getting this issue.

fish-and-bear commented 2 months ago

@andyreagan I'm not sure this got pushed to the pypi version because some people are still getting this issue.

bumping this

andyreagan commented 2 months ago

well yeah #40 will close it, but that hasn't been merged/released.

I just tagged it so that when #40 is merged, GH will automatically close this issue.

mvarnold commented 2 months ago

I guess we should really be tagging @ryanjgallagher

Could we get some other people permissions to merge?