Crash with slices e.g. Annotated[int, 3:4] #10266

Activity

1. patrick-kidger

   added

   bugmypy got something wrongmypy got something wrong

   on Mar 30, 2021
2. JelleZijlstra commented on Mar 30, 2021

   

   JelleZijlstra

   on Mar 30, 2021

   Member

   More actions

   That's a fun one! It's probably pretty easy to fix if you track down where the error comes from (probably somewhere in fastparse.py).
3. domdfcoding commented on Apr 1, 2021

   

   domdfcoding

   on Apr 1, 2021

   Contributor

   More actions

   I did some digging. The error is being raised from visit_Subscript in mypy.fastparse.TypeConverter. The code is different depending on the Python version but the error is the same.

   On Python 3.9 and later:

   mypy/mypy/fastparse.py

   Lines 1546 to 1551 in 2787d49

   	sliceval = n.slice # type: Any
   	if (isinstance(sliceval, ast3.Slice) or
   	(isinstance(sliceval, ast3.Tuple) and
   	any(isinstance(x, ast3.Slice) for x in sliceval.elts))):
   	self.fail(TYPE_COMMENT_SYNTAX_ERROR, self.line, getattr(n, 'col_offset', -1))
   	return AnyType(TypeOfAny.from_error)

   See More

   It is expecting the slice to be either:

   • an ast.Slice
   • an ast.Tuple which doesn't contain any ast.Slice nodes.

   In this case, the slice is an ast.Tuple node containing:

   • an ast.Index (the name int)
   • an ast.Slice (3:4)

   On Python 3.8 and earlier:

   mypy/mypy/fastparse.py

   Lines 1553 to 1557 in 2787d49

   	# Python 3.8 or earlier use a different AST structure for subscripts
   	if not isinstance(n.slice, Index):
   	self.fail(TYPE_COMMENT_SYNTAX_ERROR, self.line, getattr(n, 'col_offset', -1))
   	return AnyType(TypeOfAny.from_error)
   	sliceval = n.slice.value

   It is expecting the slice to be an ast.Index but instead it is an ast.ExtSlice containing:

   • an ast.Index (the name int)
   • an ast.Slice (3:4)
4. TH3CHARLie

   added

   crash

   good-first-issue

   priority-0-high

   on Apr 13, 2021
5. amrit110 commented on Apr 23, 2021

   

   amrit110

   on Apr 23, 2021

   Last edited by amrit110

   More actions

   Hi, Can I take this on? It's my very first issue! From what @domdfcoding has investigated, it seems that for py3.9, we can remove the constraint that the slice which is an ast.Tuple node not contain ast.Slice nodes. Looking at https://docs.python.org/3/library/ast.html#subscripting, I was wondering why that check was there in the first place since it says "It can be a Tuple and contain a Slice". For py3.8 and lower, the API documentation is not as clear as to what the slice can be but https://docs.python.org/3.8/library/ast.html indicates that slice is probably ast.Slice or ast.ExtSlice or ast.Index. So, for that I'm not sure, perhaps we can include that the slice can be ast.ExtSlice as well?
6. PrasanthChettri commented on Jun 6, 2021

   

   PrasanthChettri

   on Jun 6, 2021

   Contributor

   More actions

   @JelleZijlstra I am trying to fix the bug, for the example given above it's an instance of ast.Tuple(ast.Name, ast.Slice) and the problem is that of stated by @domdfcoding, and we will need to modify the conditions , it is probably not a good idea to just remove the constraint, I will need a little bit of help with the new condition to replace it with.
7. ap-- commented on Jun 27, 2021

   

   ap--

   on Jun 27, 2021

   Sponsor

   More actions

   It seems like fixing this issue in mypy.fastparse.TypeConverter.visit_Subscript would ultimately lead to allowing either both or at least one of:

   Dict[x:y]
   Dict[int, x:y]

   which is at least in one test explicitly marked to be a blocking error

   mypy/test-data/unit/check-fastparse.test

   Lines 320 to 325 in 3dcccf2

   	[case testIncorrectTypeCommentIndex]
   	from typing import Dict
   	x = None # type: Dict[x: y]
   	[out]
   	main:3: error: syntax error in type comment

   see: cd5ff81
   and: 8ac365b

   The docs for typing.Annotated https://docs.python.org/3/library/typing.html#typing.Annotated don't seem to suggest to use slicing syntax.

   @patrick-kidger It looks as if the original use case is basically trying to emulate keyword arguments in getitem similar to pep472/pep637 via the slicing notation. So maybe https://www.python.org/dev/peps/pep-0472/#pass-a-dictionary-as-an-additional-index could be an easy solution to get this working in torchtyping with only a tiny overhead in characters:

   ## the following does not throw an error ##
   # file.py
   from typing import Annotated
   def func(x: Annotated[int, {3:4}]):  # pass a dict instead of slices
      pass
   
   # command line
   mypy file.py

   Cheers,
   Andreas 😃

   P.S.: If the decision is made to lift the currently in-place slice restriction, I can prepare a PR with a fix.
8. morp007 commented on Aug 9, 2021

   

   morp007

   on Aug 9, 2021

   Last edited by morp007

   More actions

   @patrick-kidger, there's no slice in your code. See python spec: Slicings.

   Your code should look like this:

   # file.py
   from typing import Annotated
   def func(x: Annotated[int, slice(3, 4)]):
       pass

   Please, close this issue.

   👎React with 👎1Reacted by Zac Hatfield-Dodds
9. Zac-HD commented on Sep 29, 2021

   

   Zac-HD

   on Sep 29, 2021

   Contributor

   More actions

   Dict[x:y]
   Dict[int, x:y]

   IMO these should still be errors, just not (misleading) syntax error in type comment errors:

   • "expects 2 type arguments, but 1 given" like Dict[int]), with a new "did you mean..." hint for slices.
   • "Invalid type comment or annotation" like Dict[int, {1: 2}]; which already "works" for custom types like torchtyping.TensorType.

   and the tests could be updated accordingly, so that mypy continues to guide naive users towards the right way to express simple cases, without erroring out on more advanced or experimental uses.

   👍React with 👍1Reacted by Andreas Poehlmann
10. sobolevn commented on Oct 1, 2021

    

    sobolevn

    on Oct 1, 2021

    Last edited by sobolevn

    Member

    More actions

    I agree with @Zac-HD here. The error message should be improved, but Dict[int, x:y] and all of the example above should not be allowed (unless new PEPs or typing syntax changes are made). I would be super confused to see dict[x:y] as a type.

    Proposed altertives are:

    1. slice(1, 3) instead of 1:3
    2. {x: y} instead of x: y

    I will send a PR with the better error message today! 👍
11. hauntsaninja

    closed this as completedin #11241on Oct 3, 2021
12. hauntsaninja

    added a commit that references this issue on Oct 3, 2021

    Improves error message for parsing `slice` in annotations (#11241)

    ...

    This issue will close once commit eb5e086 is merged into the 'master' branch.

    eb5e086
13. Zac-HD commented on Oct 4, 2021

    

    Zac-HD

    on Oct 4, 2021

    Contributor

    More actions

    I don't think that #11241 should be considered as closing this issue without @sobolevn's proposed follow-up:

    from typing_extensions import Annotated
    
    def func(x: Annotated[int, 3:4]):  # still a fatal error, contra (?) PEP-593
        x + "a string"                 # I expect an error here but don't get one

    As above, I think it's fine for slices to be an error in stdlib generics - but IMO ought to be allowed in Annotated and third-party generic types.
14. hauntsaninja

    reopened this on Oct 4, 2021
15. JelleZijlstra

    closed this as completedin #11345on Oct 18, 2021