fix(collections): importing ABCs from collections directly is deprecated

[tvuillemin]

Changes proposed in this pull request:

• Fix the import path of the ABC classes used in the project.

Attention: @prkumar

[codecov]

Codecov Report

Merging #201 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #201   +/-   ##
=======================================
  Coverage   99.91%   99.91%           
=======================================
  Files          44       44           
  Lines        2398     2400    +2     
  Branches      176      176           
=======================================
+ Hits         2396     2398    +2     
  Misses          2        2           

Impacted Files	Coverage Δ
uplink/arguments.py	100.00% <100.00%> (ø)
uplink/auth.py	100.00% <100.00%> (ø)
uplink/commands.py	100.00% <100.00%> (ø)
uplink/converters/__init__.py	100.00% <100.00%> (ø)
uplink/converters/typing_.py	100.00% <100.00%> (ø)
uplink/decorators.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f4260d...947abde. Read the comment docs.

[prkumar]

Thanks for fixing this tech debt, @tvuillemin! Largely, these changes LGTM. Just have a few suggestions.

[prkumar]

Thanks for tackling this, @tvuillemin! I have two suggestions before approving this.

First, there are other usages of the abc module that need to be fixed. For completion, could you address these as well in your PR? Here is a GitHub query for all usages that I could find: https://github.com/prkumar/uplink/search?q=collections.Mapping+OR+collections.Iterable+OR+collections.Sequence&unscoped_q=collections.Mapping+OR+collections.Iterable+OR+collections.Sequence

Second: in general, this library follows Google's Python style guide. Accordingly, when importing submodules, the recommendation is to use from abc import collections rather than import collections.abc. Can you update the PR to follow this convention for consistency?

Thanks in advance for contributing! If you want, please add yourself to the repo's AUTHORS file in your next commit.

[tvuillemin]

Hi!

Done. I tried to respect the conventions you are using. I had a doubt though, regarding collections.namedtuple for instance. Would you rather have two import lines, to keep the collections prefix for namedtuple, or a single line from collections import abc, namedtuple and drop the prefix? I did the former in this commit, just let me know if you would have preferred the latter.

I would just like to add a small remark, if you don't mind. It felt a bit weird to write from collections import abc, for the abc prefix generally refers to the abc package of the standard library for abstract classes. Wouldn't it be slightly less confusing to use from collections.abc import Mapping and drop the prefix entirely? Just a suggestion of course; your repo, your style.

Cheers.

[prkumar]

@tvuillemin - Thanks for addressing the other usages and updating the imports to adhere to the style guide!

Regarding the import statement for namedtuple: I think keeping a separate import line looks fine to me, so no need to change in this regard.

Regarding your second point, to avoid clashing with the abc module, I think another option could be to useimport as: e.g., from collections import abc as collections_abc. I'm fine with keeping the imports as is and addressing any future conflicts as they arise. However, feel free to update to use either the alternative I described here or the from collections.abc import Mapping alternative you mentioned in your comment. I'm not too beholden to the style guide, so either works for me!

[prkumar]

Added thoughts in #201 (comment)