Making typed.List a typing Generic

[luk-f-a]

This PR adds typing information to the typed List, so in can be used as a Generic in variable annotations, e.g.

from numba.typed import List

my_list:List[int] = List()
my_list.append(1)
my_list.append('a') # would be rejected by mypy

for i in my_list:
    a: str = i # would be rejected by mypy
    print(a)

While some of these would also be runtime errors, this PR allows List to be fully checked as a generic by mypy.

This is not related to checking Numba itself with mypy, this just allows users to check their typed.List code with mypy.

Besides setting List as a Generic, it also annotates the main methods (append, getitem, setitem), and the methods for protocols: Iterable[T], Sized, Container[T], and Collection[T].

[esc]

@luk-f-a thanks for reporting this, I have added it to the review queue for now.

[luk-f-a]

@EPronovost would you have time to review this PR?

[luk-f-a]

@EPronovost thanks for the review. I fixed what you found, plus other similar things, eg other methods returning None which I hadn't annotated. I also completed those annotations that were incomplete, those where I had only annotated one input variable, like getitem, insert, index.

[luk-f-a]

Interesting finding from typeguard after I annotated typed.List

======================================================================
ERROR: test_extend_empty_unrefined (numba.tests.test_typedlist.TestExtend)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/numba/tests/test_typedlist.py", line 779, in test_extend_empty_unrefined
    l.extend(tuple())
  File "/home/vsts/miniconda3/envs/travisci/lib/python3.8/site-packages/typeguard/__init__.py", line 841, in wrapper
    check_return_type(retval, memo)
  File "/home/vsts/miniconda3/envs/travisci/lib/python3.8/site-packages/typeguard/__init__.py", line 665, in check_return_type
    raise exc from None
  File "/home/vsts/miniconda3/envs/travisci/lib/python3.8/site-packages/typeguard/__init__.py", line 663, in check_return_type
    check_type('the return value', retval, memo.type_hints['return'], memo)
  File "/home/vsts/miniconda3/envs/travisci/lib/python3.8/site-packages/typeguard/__init__.py", line 620, in check_type
    raise TypeError(
TypeError: type of the return value must be NoneType; got numba.typed.typedlist.List instead

Reproducer (it should always return None but in some cases it's returning self)

from numba.typed import List
  ...: my_list = List([1,2])
  ...: print(my_list.extend(List([1,2])))
  ...: print(my_list.extend(tuple()))
None
[1, 2, 1, 2]

[EPronovost]

I wonder if we can/should add test coverage for this beyond just using typeguard checking. For a full test of annotations, I think you need both positive and negative examples. I've recently done similar work for typeguard here. To keep it smaller scope we could also just have positive tests; i.e. a test file that uses List with annotations and gets analyzed by mypy as part of CI.
I'm curious to hear what the core devs think about this additional testing for type annotations intended to be used by users. cc: @sklam @esc @stuartarchibald
Note that if we want mypy to consider these type annotations in users' projects we need to add a py.typed file https://www.python.org/dev/peps/pep-0561/#packaging-type-information

[EPronovost]

Under strict mode inferred optionals are disabled, so these should be Optional[int]

[luk-f-a]

thanks, I didn't realize that strict optionals was the default, I thought that inferred optionals were the default.

[luk-f-a]

I wonder if we shouldn't consider switching to --no-strict-optional to avoid the added verbosity. For example, index could fit in one line (easier to read) without losing much in terms of typing (I think :int = None is clear enough). what do you think?

    def index(self, item: T, start: pt.Optional[int] = None,
              stop: pt.Optional[int] = None) -> int:

vs

def index(self, item: T, start: int = None, stop: int = None) -> int:

[EPronovost]

For internal annotations that's something we can decide on. I think for external facing annotations we should be as precise as possible to avoid incompatibilities for users who don't use --no-strict-optional.

[luk-f-a]

you're right, I changed it to the explicit Optional.

[luk-f-a]

@EPronovost I made some fixes following your latest comments, and also improved other things:

• I had only partially annotated __setitem__, now it has full annotations with overloads.
• in order to improve legibility by keeping signatures to one line, I created two type aliases.
• I ran mypy on this file and added a few fixes to remove fails
  • added # type: ignore[override]. Without this mypy complains that the method conflicts with the supertype MutableSequence (a problem with the contravariance required by Liskov's substitution principle).
  • in the annotation to extend I had to change the input from Iterable to Sequence because the method calls len(iterable) and iterable[0].

Related to this last point (running mypy), the file now passes basic tests and it could included in "level 3" files. However, I didn't actually change mypy.ini because I'd like to do it on your improved version of mypy.ini from #6222. What's the best way to do it? Wait for it to be merged or just copy and paste your version here?

[luk-f-a]

Note that if we want mypy to consider these type annotations in users' projects we need to add a py.typed file https://www.python.org/dev/peps/pep-0561/#packaging-type-information

I don't really understand this PEP. Does it mean that without a py.typed file someone with a project that imports numba would not be able to make use of the typed.Listannotation when running mypy on their project? ie mypy would ignore the annotations?

Update: after investigating and asking in the typing gitter, I confirmed that it is true that " without a py.typed file someone with a project that imports numba would not be able to make use of the typed.Listannotation when running mypy on their project"

[EPronovost]

Sorry for the delay. Yes to your above question. There's relevant code in the mypy module finder here: https://github.com/python/mypy/blob/master/mypy/modulefinder.py#L180-L194

[sklam]

typeguard is failing for numba.tests.test_typedlist. On CI, the test slices didn't run any of the failing tests by "luck".

[sklam]

I read #5958 (comment) that it needs to be a Sequence. But, typeguard is complaining that numpy.ndarray is not a Sequence for 5 tests under numba.tests.test_typedlist.TestListSort. pt.Iterable[T] would work for typeguard but mypy would complain about the __len__. The following seems to work for both checkers:

T_co = pt.TypeVar('T_co', covariant=True)
class _Sequence(pt.Protocol[T_co]):
    def __getitem__(self, i: int) -> T_co: ...
    def __len__(self) -> int: ...

[sklam]

Test is failing because Protocol is also new on py3.8.

[luk-f-a]

typeguard is failing for numba.tests.test_typedlist. On CI, the test slices didn't run any of the failing tests by "luck".

what's the solution? Did you run the full suite locally? Is it possible to expand typeguard to run on more slices?

[luk-f-a]

Test is failing because Protocol is also new on py3.8.

I tried a fix but made it worse. I'll build something better tomorrow.

[sklam]

what's the solution? Did you run the full suite locally? Is it possible to expand typeguard to run on more slices?

Right, we will need to make more configs use typeguard.

[sklam]

If we guard the protocol type for py3.8 only and use a string to refer to it, we can avoid changing the implementation of extend.

something like

if python_version >= (3, 8):
    class _Sequence(pt.Protocol[T_co]): ...

class List:
    def extend(self, iterable: "_Sequence"): ...

[sklam]

I think it's ok to do type-checks on only py>=3.8. I'm seeing that Protocol is essential.

[luk-f-a]

Thanks @sklam, I'll try what you suggested above. Funny you found a 5th option, I was considering the pros/cons of:

• keeping extend untyped until 3.8 is the minimum supported version
• changing extend such that it does not require len and getitem (I tried this yesterday and failed) so it can be types as Iterable
• requiring typing_extensions as a dependency (cons: not worth adding a dependency for this)
• typing with _Sequence but not with _Sequence[T], and _Sequence is Any for <3.8