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
MAINT: Cleanup vecdot
's signature, typing, and importing
#26313
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
* As `numpy.vecdot` is now a ufunc it has a less precise signature. | ||
This is due to the limitations of ufunc's typing stub. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -497,39 +497,6 @@ def tensordot( | |
axes: int | tuple[_ShapeLike, _ShapeLike] = ..., | ||
) -> NDArray[object_]: ... | ||
|
||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeUnknown, x2: _ArrayLikeUnknown, axis: int = ... | ||
) -> NDArray[Any]: ... | ||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeBool_co, x2: _ArrayLikeBool_co, axis: int = ... | ||
) -> NDArray[np.bool]: ... | ||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeUInt_co, x2: _ArrayLikeUInt_co, axis: int = ... | ||
) -> NDArray[unsignedinteger[Any]]: ... | ||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeInt_co, x2: _ArrayLikeInt_co, axis: int = ... | ||
) -> NDArray[signedinteger[Any]]: ... | ||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeFloat_co, x2: _ArrayLikeFloat_co, axis: int = ... | ||
) -> NDArray[floating[Any]]: ... | ||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeComplex_co, x2: _ArrayLikeComplex_co, axis: int = ... | ||
) -> NDArray[complexfloating[Any, Any]]: ... | ||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeTD64_co, x2: _ArrayLikeTD64_co, axis: int = ... | ||
) -> NDArray[timedelta64]: ... | ||
@overload | ||
def vecdot( | ||
x1: _ArrayLikeObject_co, x2: _ArrayLikeObject_co, axis: int = ... | ||
) -> NDArray[object_]: ... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can these type annotations be deleted? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah because vecdot is a ufunc now and ufunc.pyi handles it for us, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! For example there is no |
||
|
||
@overload | ||
def roll( | ||
a: _ArrayLike[_SCT], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,16 @@ assert_type(np.matmul.identity, None) | |
assert_type(np.matmul(AR_f8, AR_f8), Any) | ||
assert_type(np.matmul(AR_f8, AR_f8, axes=[(0, 1), (0, 1), (0, 1)]), Any) | ||
|
||
assert_type(np.vecdot.__name__, Literal["vecdot"]) | ||
assert_type(np.vecdot.ntypes, Literal[19]) | ||
assert_type(np.vecdot.identity, None) | ||
assert_type(np.vecdot.nin, Literal[2]) | ||
assert_type(np.vecdot.nout, Literal[1]) | ||
assert_type(np.vecdot.nargs, Literal[3]) | ||
assert_type(np.vecdot.signature, Literal["(n),(n)->()"]) | ||
assert_type(np.vecdot.identity, None) | ||
assert_type(np.vecdot(AR_f8, AR_f8), Any) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a bunch of other tests like this that got deleted. Why only this one now? Or just an oversight? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would improve my confidence that the typing changes are correct if all the tests that used to be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests from As of now If we want to keep removed tests then there's a question how we want to proceed: Remove GUFunc based typing stub and restore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can you explain a little more why e.g. If so, it sounds like the hand-written type overrides for vecdot should be used and similar overrides should be written for matmul, or the automatically generated typing should be improved to capture the semantics of the old hand-written types. Users might write types using those in numpy 2.0 (unless you're looking to backport this to 2.0, which might not be possible given timing) so we shouldn't make the types less expressive or possibly break typing tests by changing semantics. I'll defer to @BvB93 or others who are more experienced dealing with types about all this though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC it's a limitation of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is an unfortunate limitation of callables that are not just normal python functions: the latter are somewhat unique in that you it is very easy the customize the signature of each individual function at the instance-level; a degree of flexibility that is in pretty much all other cases only possible at the class-level. Unfortunately (in this context) ufuncs have a bunch of extra methods in attributes, none of which can be adequately typed with normal python functions (as was the case prior to this PR) and thus forcing the use of a single ufunc class (or small set, as we do here with these typing-only nin-/nout-based ufunc classes). The end result is that, unless you're willing to go through the massive task of creating a single ufunc class with customized signature for every single ufunc, that you're stuck with a less precise signature, one that is unfortunately too imprecise to actually infer the output type of the likes of |
||
|
||
assert_type(np.bitwise_count.__name__, Literal['bitwise_count']) | ||
assert_type(np.bitwise_count.ntypes, Literal[11]) | ||
assert_type(np.bitwise_count.identity, None) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,6 @@ array_api_tests/test_signatures.py::test_func_signature[reshape] | |
array_api_tests/test_signatures.py::test_func_signature[argsort] | ||
array_api_tests/test_signatures.py::test_func_signature[sort] | ||
|
||
# TODO: check why in CI `inspect.signature(np.vecdot)` returns (*arg, **kwarg) | ||
# instead of raising ValueError. mtsokol: couldn't reproduce locally | ||
# ufuncs signature on linux is always <Signature (*args, **kwargs)> | ||
# np.vecdot is the only ufunc with a keyword argument which causes a failure | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do the array API tests also check that vecdot's signature is correct using duck typing? Probably worth doing both - check the signature and validate that the signature is correct! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is |
||
array_api_tests/test_signatures.py::test_func_signature[vecdot] |
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.
I wanted
vecdot
to be closer tomatmul
in imports/__all__
as they are both gufuncs.