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

"Tuple expression not allowed in type annotation" error when pyright type checking is disabled #5451

Closed
DetachHead opened this issue Jul 10, 2023 · 13 comments
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request

Comments

@DetachHead
Copy link

i'm using an alternate type checker (basedmypy) that supports additional features such as tuple literal types, which is why i have pylance type checking disabled. however it seems that some errors such as this one are still displayed regardless of that setting

Environment data

  • Language Server version: 2023.6.40 (pyright 3f84540)
  • OS and version: windows 10
  • Python version (& distribution if applicable, e.g. Anaconda): 3.11

Code Snippet

foo: (int, str) = (1, "")

Repro Steps

set "python.analysis.typeCheckingMode" to "off" in settings.json

Expected behavior

no error

Actual behavior

Tuple expression not allowed in type annotation
  Use Tuple[T1, ..., Tn] to indicate a tuple type or Union[T1, T2] to indicate a union type
@erictraut
Copy link
Collaborator

Could someone from the pylance team please transfer this to pyright, since it's a core type checking issue? Thanks!

@rchiodo rchiodo transferred this issue from microsoft/pylance-release Jul 10, 2023
@erictraut
Copy link
Collaborator

I don't consider this a bug, but I'll treat it as a feature request. A tuple expression is not valid within a type annotation, so pyright is correct to generate an error here. This isn't a type violation; it's akin to a syntax error, since this syntax is not valid for annotations. For that reason, it's not currently disabled when type checking is disabled.

I presume that you're using pylance for its language server features. These features leverage type annotations if present, so if you're using invalid type annotation forms, the language server features in pylance will not work in those cases. If you want your code to work with other tools, you should stick with valid annotation forms.

I think it's unfortunate that basedmypy has chosen to support non-standard annotation expression forms. This will create friction with other tools that are standards-compliant. I encourage you to work with the typing community to standardize features that you want to see in the Python type system rather than creating non-standard variants.

It was relatively easy to move this diagnostic to the reportGeneralTypeIssue diagnostic rule so it can be disabled, so I've made that change. It will be included in the next release of pyright and a future release of pylance.

@erictraut erictraut added enhancement request New feature or request addressed in next version Issue is fixed and will appear in next published version labels Jul 11, 2023
@DetachHead
Copy link
Author

DetachHead commented Jul 12, 2023

I presume that you're using pylance for its language server features. These features leverage type annotations if present, so if you're using invalid type annotation forms, the language server features in pylance will not work in those cases. If you want your code to work with other tools, you should stick with valid annotation forms.

that's fair. i'm not asking for pylance to support non-standard type annotations. all i want is for it to not show errors on them when i've explicitly disabled pylance's type checking

@KotlinIsland
Copy link

I think it's unfortunate that basedmypy has chosen to support non-standard annotation expression forms. This will create friction with other tools that are standards-compliant. I encourage you to work with the typing community to standardize features that you want to see in the Python type system rather than creating non-standard variants.

Basedmypy intentionally has non-standard and breaking changes, it's intended to be as based as possible to promote beneficial changes within the typing ecosystem.

@PabloLION
Copy link

PabloLION commented Jul 12, 2023

To me the (int, str) seems equivalent to tuple([int,str]) not tuple[int,str]. Maybe it's good to reduce the verbosity like | in PEP-604, but in some case we do need tuple of types and it can be confusing and might(?) break the backward compatibility. An example usecase of tuple([int,str]) would be

def validate_type(
    x: tuple[int, str] | tuple[int, bytes] | tuple[int, bytearray]
) -> bool:
    # check if x is a tuple of length 2 and the first element is an int
    # and the second element is a str, bytes or bytearray
    accpeted_types = {(int, str), (int, bytes), (int, bytearray)}
    for t in accpeted_types:
        for x_ele, t_ele in zip(x, t, strict=True):
            if not isinstance(x_ele, t_ele):
                break
        else:
            return True
    return False


# test for validate_type
assert validate_type((1, "a")) == True
assert validate_type((1, b"a")) == True
assert validate_type((1, bytearray(b"a"))) == True
assert validate_type((1, 1)) == False
assert validate_type((1, 1.0)) == False
a worse example
def convert(x: int | str) -> str | int:
    # Convert x to a string if it is an integer, or to an integer if it is a string.
    valid_types = (int, str)
    if type(x) not in valid_types:
        raise TypeError(f"Expected type {valid_types}, got {type(x)}")
    if type(x) == int:
        return str(x)
    else:
        return int(x) # raise ValueError if x is not a valid integer


# test of convert
assert convert(1) == "1"
assert convert("1") == 1

@erictraut
Copy link
Collaborator

Basedmypy intentionally has non-standard and breaking changes

This is fine in areas where you don't violate the typing standard or the conventions that have been agreed upon by all major type checkers. In cases where you diverge from the standard, you are creating unnecessary friction with other tools. Please work within the typing community to standardize such changes or stick with the standards if you can't achieve consensus. This may require a bit more work on your part than sitting on the side and maintaining your own (incompatible) fork of mypy, but it's the right thing to do for the community.

@DetachHead
Copy link
Author

It was relatively easy to move this diagnostic to the reportGeneralTypeIssue diagnostic rule so it can be disabled, so I've made that change. It will be included in the next release of pyright and a future release of pylance.

thanks :) btw i believe there are many more errors that could be moved as well (i assume microsoft/pylance-release#2982 was the same issue). imo only errors that prevent any further analysis of the file (eg. syntax errors) should be displayed when type checking is disabled. i'll raise separate issues as i encounter them

not only for the purpose of using non-standard type checkers, there are other situations where i've had code that normal mypy was happy with but pyright wasn't

@PabloLION

This comment was marked as off-topic.

@DetachHead
Copy link
Author

This is fine in areas where you don't violate the typing standard or the conventions that have been agreed upon by all major type checkers. In cases where you diverge from the standard, you are creating unnecessary friction with other tools. Please work within the typing community to standardize such changes or stick with the standards if you can't achieve consensus. This may require a bit more work on your part than sitting on the side and maintaining your own (incompatible) fork of mypy, but it's the right thing to do for the community.

weren't type hints intentionally designed to allow them to be used in non-standard ways?

https://peps.python.org/pep-0484/#abstract:

Note that this PEP still explicitly does NOT prevent other uses of annotations, nor does it require (or forbid) any particular processing of annotations, even when they conform to this specification.

a specific example is the @no_type_check decorator:

from typing import no_type_check


@no_type_check
def foo(a: (int, str)) -> None:  # still errors
    ...

@PabloLION
Copy link

PabloLION commented Jul 12, 2023

@DetachHead PEP-484 was kinda old (29-Sep-2014) and it says "this PEP(484) still explicitly does NOT prevent other ..." .
And I saw in PEP 695 – Type Parameter Syntax Eric wrote

"There is consensus within the Python static typing community that it is time to provide a formal syntax that is similar to other modern programming languages that support generic types."

@erictraut
Copy link
Collaborator

PEPs are point-in-time design documents. They are not living documentation, and they are not typically updated when they are superseded or aspects are clarified by agreement among the major type checkers. PEP 484 was intentionally vague on many aspects of the typing system, and many of those value parts have been clarified in the many years since PEP 484 was written.

The use of tuple literals in type annotations is one such example. There is a well-defined and standardized way to specify tuple types using tuple[x, y z]. If you're going to deviate from the standards, you shouldn't expect other tools in the ecosystem to accommodate you.

@DetachHead
Copy link
Author

its hard to know what the current consensus is on stuff like this when none of the existing documentation has been updated. if the PEPs don't get updated fair enough (though it would be nice if there was a message at the top mentioning that parts of it were superseeded in later PEPs), but i would at least expect a deprecation message on @no_type_check here if using it in the current year is frowned upon.

@erictraut
Copy link
Collaborator

This is included in pyright 1.1.317, which I just published. It will also be included in future releases of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants