Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImportError: cannot import name 'Mapping' from 'collections' #113

Closed
Phlya opened this issue Feb 28, 2022 · 3 comments
Closed

ImportError: cannot import name 'Mapping' from 'collections' #113

Phlya opened this issue Feb 28, 2022 · 3 comments

Comments

@Phlya
Copy link
Member

Phlya commented Feb 28, 2022

Above error appears in python 3.10. As usual with collections, import should be fixed with this change: from collections.abc import Mapping.
I haven't checked from which version of python this would work and whether we can just simply change it or it would break in earlier versions.

File /tungstenfs/scratch/ggiorget/ilya/condaenvs/hic/lib/python3.10/site-packages/pairtools/__init__.py:116, in <module>
    113         return func(*args, **kwargs)
    114     return wrapper
--> 116 from .pairtools_dedup import dedup
    117 from .pairtools_sort import sort
    118 from .pairtools_flip import flip

File /tungstenfs/scratch/ggiorget/ilya/condaenvs/hic/lib/python3.10/site-packages/pairtools/pairtools_dedup.py:14, in <module>
     12 from . import _dedup, _fileio, _pairsam_format, _headerops, cli, common_io_options
     13 from .pairtools_markasdup import mark_split_pair_as_dup
---> 14 from .pairtools_stats import PairCounter
     17 UTIL_NAME = 'pairtools_dedup'
     19 # you don't need to load more than 10k lines at a time b/c you get out of the 
     20 # CPU cache, so this parameter is not adjustable

File /tungstenfs/scratch/ggiorget/ilya/condaenvs/hic/lib/python3.10/site-packages/pairtools/pairtools_stats.py:10, in <module>
      6 import click
      8 import numpy as np
---> 10 from collections import OrderedDict, Mapping
     12 from . import _fileio, _pairsam_format, cli, _headerops, common_io_options
     14 UTIL_NAME = 'pairtools_stats'

ImportError: cannot import name 'Mapping' from 'collections' (/tungstenfs/scratch/ggiorget/ilya/condaenvs/hic/lib/python3.10/collections/__init__.py)
@Phlya
Copy link
Member Author

Phlya commented Mar 2, 2022

I fixed it in #107
Also there is no OrderedDict in recent (3.10?) python, so I just changed it all to regular dicts, since they are now ordered.

@agalitsyna
Copy link
Member

Hi, OrderedDict is present in Python 3.10 and even in 3.11. It is actually actively maintained and constantly improved. E.g. it was recently made 2x times faster.
You can simply do from collections import OrderedDict as it was before.

Another point is that I benchmarked OrderedDict against dict, and simple iteration over d.items() is ~3-5 times slower for OrderedDict (Python 3.10). Adding to the dict/accessing by the key have the same performance on simple test set.

Just to make sure: we do not support Python below 3.7, so it's safe to replace the OrderedDict by dict (which started to be ordered from Python 3.7)?

Ipersonally like the idea to stick to an old implementation that makes the "ordering" feature explicit. But it's up to you. If you decide to remove OrderedDict, please, fix also the documentation, now there are many mentions of OrderedDict class in the docstrings.

@golobor
Copy link
Member

golobor commented Mar 7, 2022

i second the idea to move from OrderedDict to dict, as ordering is now guaranteed in dicts : https://stackoverflow.com/questions/39980323/are-dictionaries-ordered-in-python-3-6

@agalitsyna agalitsyna mentioned this issue Apr 6, 2022
31 tasks
agalitsyna added a commit that referenced this issue Apr 7, 2022
* Removed OrderedDict remnants: #113

* Pairtools dedup fix of cython forgotten line from recent PR:
#107

* header add_columns function

* pairtools restrict: header improvement

* pairtools stats: warning of dataframe slicing fix

* np.genfromtxt warning resolved:
#71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants