Add typing to array.py#478
Merged
bryevdv merged 18 commits intonv-legate:branch-22.10from Aug 16, 2022
bryevdv:bryanv/mypy_array_py
Merged
Add typing to array.py#478bryevdv merged 18 commits intonv-legate:branch-22.10from bryevdv:bryanv/mypy_array_py
bryevdv merged 18 commits intonv-legate:branch-22.10from
bryevdv:bryanv/mypy_array_py
Conversation
bryevdv
commented
Jul 28, 2022
|
|
||
| [[tool.mypy.overrides]] | ||
| module = ["cunumeric.array"] | ||
| ignore_errors = true |
Contributor
Author
There was a problem hiding this comment.
We could leave an empty list here as a convenience in case new modules need to be temp ignored in the future
manopapad
reviewed
Aug 15, 2022
| def mean( | ||
| self, | ||
| axis: Any = None, | ||
| dtype: Union[np.dtype[Any], None] = None, |
Contributor
There was a problem hiding this comment.
Noting this as future work: Any public functions in array.py and module.py should accept npt.DTypeLike, e.g. they should accept things like int and np.float64. Internal functions, mostly in thunk.py, eager.py and deferred.py (and sometimes in array.py and module.py) can be constrained to only np.ndtype[Any], with the frontend functions converting as appropriate.
Value of this type are eventually fed to np.can_cast, which doesn't accept None.
Doing the former can result in unexpected behavior, in the common case where one value is a proper np.dtype object, while the other is something that is not itself a np.dtype, but something convertible to it: >>> np.dtype(np.int64) is np.int64 False >>> np.dtype(np.int64) == np.int64 True Some of the uses in the modified code were actually safe, because a pre-existing array's dtype is always wrapped in a np.dtype object, but I changed them too, for the sake of consistency.
It doesn't matter now, but we might have forgotten to change it to the more general signature when we got around to implementing this.
manopapad
approved these changes
Aug 15, 2022
Contributor
manopapad
left a comment
There was a problem hiding this comment.
I made some small drive-by fixes as I was reading through the changes in array.py. please take a look at my commits if you have time. Otherwise LGTM.
sbak5
pushed a commit
to sbak5/cunumeric
that referenced
this pull request
Aug 17, 2022
* checkpoint array * Clean up cunumeric.tile * Disallow kind=None on cn.ndarray.argsort, to match cn.argsort * Avoid a cast * Update a docstring to match the inferred type signature * Fix handling of out=cn.ndarray in clip * add new missing types * Values of type CastingKind should never be None Value of this type are eventually fed to np.can_cast, which doesn't accept None. * Use np.ndarray.tobytes over the deprecated tostring * Minor fixes * Don't compare dtypes with `is`, but with == Doing the former can result in unexpected behavior, in the common case where one value is a proper np.dtype object, while the other is something that is not itself a np.dtype, but something convertible to it: >>> np.dtype(np.int64) is np.int64 False >>> np.dtype(np.int64) == np.int64 True Some of the uses in the modified code were actually safe, because a pre-existing array's dtype is always wrapped in a np.dtype object, but I changed them too, for the sake of consistency. * No need to call dtype.type if using == * Fix dtype= handling in _diag_helper * Copy NumPy's type signature for an unimplemented function It doesn't matter now, but we might have forgotten to change it to the more general signature when we got around to implementing this. Co-authored-by: Manolis Papadakis <manopapad@gmail.com>
manopapad
pushed a commit
that referenced
this pull request
Nov 17, 2024
XiaLuNV
added a commit
to XiaLuNV/cunumeric
that referenced
this pull request
Dec 13, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR adds annotations to
array.pyremoving the final "ignored" module formypy.Some caveats:
The work concentrates on documenting the existing codebase, and only make improvements that did not require extensive code changes. Sometimes this meant punting with
Anytypes for the time being. Specifically, there are multiple different sets of typs in use for "axis" and "axes" etc, that do no always seem 100% compatible. This work should be handled in a dedicated PR since code changes may be advised in several places to being everything to consistency.Other specific comments left inline.