chore(hardware): upgrade numpy #9880
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bumping numpy revealed some real issues with our code and also some real issues with numpy's as-yet incomplete typing, such as - many functions particularly in numpy.linalg aren't typed, like `norm` - many arithmetic ops and functions behave quite strangely, often returning a `np.Float[_64bit, Any]` instead of a float64 equivalent - ndarray typing is still not quite there, but that's mostly because some typing peps need to be merged first This should be a low risk set of changes.
Codecov Report
@@ Coverage Diff @@
## edge #9880 +/- ##
==========================================
- Coverage 75.20% 75.00% -0.21%
==========================================
Files 2005 2007 +2
Lines 53156 53293 +137
Branches 5148 5148
==========================================
- Hits 39976 39971 -5
- Misses 12161 12303 +142
Partials 1019 1019
Flags with carried forward coverage won't be shown. Click here to find out more.
|
mcous
approved these changes
Apr 5, 2022
| @@ -16,6 +16,9 @@ | |||
| vectorize, | |||
| ) | |||
|
|
|||
| if TYPE_CHECKING: | |||
There was a problem hiding this comment.
If we want to get rid of this annoying Codecov message, we should copy over the .coveragerc from api to ignore if TYPE_CHECKING: blocks
[report]
exclude_lines =
@overload
if TYPE_CHECKING:
| @@ -484,5 +490,5 @@ def unit_vector_multiplication( | |||
| unit_vector: Coordinates[AxisKey, np.float64], value: np.float64 | |||
| ) -> Coordinates[AxisKey, np.float64]: | |||
| """Multiply coordinates type by a float value.""" | |||
| targets: np.ndarray = vectorize(unit_vector) * value | |||
| targets: "NDArray[np.float64]" = vectorize(unit_vector) * value | |||
There was a problem hiding this comment.
Should we prefer from __future__ import annotations over string-types, where possible?
Laura-Danielle
approved these changes
Apr 6, 2022
ecormany
pushed a commit
that referenced
this pull request
Apr 8, 2022
Bumping numpy revealed some real issues with our code and also some real issues with numpy's as-yet incomplete typing, such as - many functions particularly in numpy.linalg aren't typed, like `norm` - many arithmetic ops and functions behave quite strangely, often returning a `np.Float[_64bit, Any]` instead of a float64 equivalent - ndarray typing is still not quite there, but that's mostly because some typing peps need to be merged first This should be a low risk set of changes. Co-authored-by: Mike Cousins <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Building on #9871 and other work by @mcous , bump the version of numpy used in hardware to 1.21.1 which matches what will be on the OT-2 and has some better typing.
Bumping numpy revealed some real issues with our code and also some real
issues with numpy's as-yet incomplete typing, such as
normreturning a
np.Float[_64bit, Any]instead of a float64 equivalentsome typing peps need to be merged first
This should be a low risk set of changes.