-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Bump mypy to latest version #5767
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type check test is failing
type check test is failing as it turns out
Yeah, there seem to be differences between running mypy in my local environment and in CI, so I missed these failures locally. I'll take a look to see how tricky these errors are to fix. |
@@ -119,7 +119,7 @@ def inner_product_of_state_and_x(self, x: int) -> Union[float, complex]: | |||
* 2 ** (-sum(self.v) / 2) | |||
* 1j**mu | |||
* (-1) ** sum(self.v & u & self.s) | |||
* np.all(self.v | (u == self.s)) | |||
* bool(np.all(self.v | (u == self.s))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use (...).item()
to turn numpy thing into python thing?
This sounds worrying. Do you have any idea why? |
Yeah I had this too, and was a very recent install. Maybe because our python versions are higher? |
@dabacon, the problem in my case was an older version of numpy without type annotations. I think it's working now. |
I think I've got everything now; PTAL @mpharrigan, @dabacon. I found a smattering of random lint errors that were revealed when touching various files. FWIW I think we should change CI to lint everything, because the current incremental linting means there can be lint errors hiding in a file that have to be handled by the next person to touch the file for unrelated reasons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look like reasonable changes to me.
Automerge cancelled: A required status check is not present. Missing statuses: ['Build docs', 'Build protos', 'Coverage check', 'Doc test', 'Format check', 'Misc check', 'Notebook formatting', 'Pytest MacOS (3.7)', 'Pytest MacOS (3.8)', 'Pytest MacOS (3.9)', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Type check', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage'] |
Prompted by discussion on #5486; this turned out to be
not as difficult as I thoughtsomewhat difficult.