Skip to content

New error code for slices syntax, refs #10266 #11279

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

Closed
wants to merge 2 commits into from
Closed

New error code for slices syntax, refs #10266 #11279

wants to merge 2 commits into from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 6, 2021

Now Dict[x:y] can be silently skipped and treated as Any if --disable-error-code slice-syntax is specified.
I hope that this is what our users with custom plugins are looking for.

Closes #10266
Refs #11241

CC @Zac-HD

@JelleZijlstra
Copy link
Member

I don't think this is a great solution. Here's my understanding:

  • Users want to use slices in Annotated (Annotated[int, 3:4]).
  • Nobody wants to use Dict[x:y]
  • But mypy implementation details mean the two cases are handled in the same place

So I think we should allow the Annotated variant (people should be able to put whatever they want in Annotated), and keep erroring on Dict[x:y]. A new error code doesn't make sense to me because this is such a highly specific case, and if we fix Annotated it's difficult to imagine a use case for disabling the new error code.

Is it difficult in the code to allow slices in Annotated but not elsewhere?

@sobolevn
Copy link
Member Author

sobolevn commented Oct 6, 2021

We need @Zac-HD here 🙂

Do we really need to only support Annotation? Or should this be allowed for any type annotations including custom ones?

@Zac-HD
Copy link
Contributor

Zac-HD commented Oct 6, 2021

Here's my desired behaviour:

from typing import Annotated, Dict
from torchtyping import TensorType

a: Annotated[int, 1:2]             # no error
b: Dict[int, x:y]                  # as for Dict[int, slice(x,y)]
c: Dict[x:y]                       # as for Dict[slice(x,y)]
t: TensorType["batch":..., float]  # no error - important to support third-party experimentation

And those errors, with my desired suggestions, are:

error: Invalid type comment or annotation  [valid-type]
note:  Suggestion: use slice[...] instead of slice(...)  # use slice[...] instead of `:` slice notation

error: "dict" expects 2 type arguments, but 1 given  [type-arg]
error: Invalid type comment or annotation  [valid-type]
note:  Suggestion: use slice[...] instead of slice(...)  # use `x, y` instead of `:` slice notation

As far as I can tell, the original motivation for erroring out on slices was to direct users from Dict[x:y] to Dict[x, y] - and at the time there was no Annotated type or third-party experiments to cause problems for.

In priority order, it's most important for me that a and t should have no error (that would solve this for me!); then IMO b and c should emit valid-type errors (but no big deal since I'd fix them either way); and finally the improved suggestions would be nice to have if they're not too hard to implement.

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

Successfully merging this pull request may close these issues.

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