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

False report (unsupported-assignment-operation) on plain typing.Counter usage #2456

Closed
jhbuhrman opened this issue Aug 30, 2018 · 3 comments
Closed

Comments

@jhbuhrman
Copy link

Steps to reproduce

  1. Run pylint (missing-docstring disabled) over this file (named typed_counter_pylint_check.py, find attached, remove the .txt suffix - typed_counter_pylint_check.py.txt):
    import collections
    import typing
    
    SOME_WORDS = ['spam', 'ham', 'eggs', 'ham']
    
    def untyped() -> None:
        word_hist_untyped = collections.Counter()
        for word in SOME_WORDS:
            word_hist_untyped[word] += 1
        assert word_hist_untyped == dict(spam=1, ham=2, eggs=1)
    
    def typed() -> None:
        word_hist_typed = typing.Counter[str]()
        for word in SOME_WORDS:
            word_hist_typed[word] += 1
        assert word_hist_typed == dict(spam=1, ham=2, eggs=1)
    
    untyped()
    typed()

Current behavior

$ pylint --disable=missing-docstring typed_counter_pylint_check.py
************* Module typed_counter_pylint_check
typed_counter_pylint_check.py:16:8: E1137: 'word_hist_typed' does not support item assignment (unsupported-assignment-operation)

------------------------------------------------------------------
Your code has been rated at 6.67/10 (...)

Expected behavior

$ pylint --disable=missing-docstring typed_counter_pylint_check.py
Your code has been rated at 10.00/10 (...)

pylint --version output

pylint 2.1.1
astroid 2.0.4
Python 3.7.0 (default, Jun 29 2018, 20:13:13)
[Clang 9.1.0 (clang-902.0.39.2)]

Note

This issue might be related to #2420.

@PCManticore
Copy link
Contributor

This is caused by our brain plugin for the typing module, which overwrites the classes from the typing module to be easier to understand as generic types. But I don't think your use case is valid though, these classes are not intended to be used as concrete classes but rather as types. I don't think it's worth fixing it on our side.

@jhbuhrman
Copy link
Author

jhbuhrman commented Aug 31, 2018

@PCManticore Although your statement is true for many of the items in typing, some of the classes in typing are meant as a replacement. One (other) example is typing.NamedTuple as a replacement for collections.namedtuple.

The proof of the pudding is whether objects can be instantiated or not. For example you can't instantiate from typing.List, but you can instantiate from typing.NamedTuple. As you run the example above, you can see that one can also instantiate from typing.Counter and the code runs as expected.

So I would advocate to have this issue reopened.

@jhbuhrman
Copy link
Author

jhbuhrman commented Sep 3, 2018

After having discussed this on python/typing - Gitter I came to understand that instantiating directly from typing.Counter[str] is 'discouraged'. The preferred way is to import both collections.Counter and typing.Counter and use the typing variant only for the type annotations. Another consequence is that I have to explicitly have to annotate the word_hist_typed variable, as in

    ...
    word_hist_typed: typing.Counter[str] = collections.Counter()
    ...

This indeed isn't pylint's primary concern. So I withdraw my plea to have this issue reopened ;-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants