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

mypy type check errors with default argument to cirq protocols. #4992

Open
2 tasks
tanujkhattar opened this issue Feb 14, 2022 · 1 comment
Open
2 tasks
Labels
area/mypy area/protocols kind/bug-report Something doesn't seem to work. priority/p2 Next release should contain it triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@tanujkhattar
Copy link
Collaborator

Description of the issue
This came up while reviewing #4881 and offline discussions with @Zshan0.

A common pattern used throughout cirq protocols is to take a default: TDefault parameter which should be returned if the protocol does not succeed, or raise a type error if no explicit default has been provided. For example:

RaiseTypeErrorIfNotProvided: np.ndarray = np.array([])
TDefault = TypeVar('TDefault')

def unitary(
val: Any, default: TDefault = RaiseTypeErrorIfNotProvided
) -> Union[np.ndarray, TDefault]:

However, this construct actually raises a mypy error and is a known issue -- python/mypy#8739

How to reproduce the issue

Here is a minimal failing example that follows a similar construct:

TDefault = TypeVar('TDefault')
RaiseTypeErrorIfNotProvided: List = []

def unitary(val: Any, default: TDefault = RaiseTypeErrorIfNotProvided) -> Union[List, TDefault]:
    return default

https://mypy-play.net/?mypy=latest&python=3.10&gist=f60ce6c21997d6559f0bf5bbe0881230

Summary
There are two different issues here that need to fixed:

  • Figure out and fix why ./check/mypy does not detect this error.
  • Fix the type annotations on default parameter of protocols. (eg: by using the overload + optional signature for default).

Cirq version
0.14dev

@MichaelBroughton MichaelBroughton added the triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on label Feb 16, 2022
@95-martin-orion 95-martin-orion added the priority/p2 Next release should contain it label Feb 16, 2022
@95-martin-orion
Copy link
Collaborator

Cirq cynq: not terribly high-priority, but we should at least confirm that mypy isn't skipping files/directories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mypy area/protocols kind/bug-report Something doesn't seem to work. priority/p2 Next release should contain it triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

No branches or pull requests

4 participants