signal.windows: Add type stubs for _windows.pyi.
#153
Conversation
|
There are some merge conflicts |
There was a problem hiding this comment.
Thanks for this; it's good to see some shape-typing in action 🎉.
I noticed that the sym parameters are currently typed as a bool, just like the docs say it should be. But I believe that in practice, it could be anything that can be used within an if statements, or passed as argument to a bool.
So in similar cases like these, I've recently been using optype.CanBool as annotation (implementation). And while theoretically any object should be acceptable (object doesn't implement __bool__), I thought that annotating them as _: CanBool would be a more informative than _: object, given the name and all 🤷🏻.
In cases where I had to overload on "truthy" and "falsy" values, I usually went for Literal[0, False] and Literal[1, True], as there seems to be a trend within the scipy codebase of using 0 instead of False, and 1 instead of True.
I am actually really confused about this. My point of my commit was to change this file so of course it would conflict, its what I am trying to change! Must be a git quirk or I just forgot something about how git works. |
|
The last change made to |
Okay that makes sense. I think I messed up my branch creation command because I meant to branch off the upstream master using something like |
|
I often mess it up with git as well; I mostly rely on some vscode plugins for that kinda stuff haha |
In `_windows.pyi`. Makes it less verbose.
In `_windows.pyi`.
In `_windows.pyi`.
In `_windows.pyi`.
In `_windows.pyi`'s `dpss` function overloads
In `_windows.pyi`.
|
I think I addressed everything except not so sure about the overload semantics for the |
|
It might be helpful to write a simple type-test for |
In `_windows.pyi`.
`dpss`. In `_windows.pyi` and the `dpss` function. Allow callers to use positional or keyword argument passing for `Kmax` and `return_ratios`.
This function has a few different overloads so it is nice to have tests for them.
|
@jorenham Thanks! The explanation on overloads made a lot of sense. Hopefully the overloads make sense now. Added some tests for |
In `_windows.pyi`. This is because `ndarray` is not a `Sequence`.
pavyamsiri
force-pushed
the
improve/windows
branch
from
de9b247 to
8a3965a
Compare
| _Weights: TypeAlias = ( | ||
| Sequence[AnyReal] | ||
| | np.ndarray[tuple[int], np.dtype[np.floating[Any]]] | ||
| | np.ndarray[tuple[int], np.dtype[np.integer[Any]]] | ||
| | np.ndarray[tuple[int], np.dtype[np.bool_]] | ||
| ) |
There was a problem hiding this comment.
As much as I am a fan of shape-typed arrays, I'm afraid that we can't require specific shapes for input (for now), because numpy has very little support for it.
So for example even simple things like np.array([1]) currently return np.ndarray[tuple[int, ...]] (although I believe that I fixed that, but that'll be in the upcoming 2.2.0 release, and we currently have to support numpy>=1.24).
tldr; for output it's perfectly fine to use shape-typing, but for input it's too restrictive at the moment.
There was a problem hiding this comment.
Okay got it I will change it.
In `_windows.pyi`.
|
thanks @pavyamsiri ! |
Contributes to completing #99.
This PR adds type stubs for the module
scipy.signal.windows._windows.The function overloads are bit dodgy though and I could have maybe used
AnyIntinstead ofintfor some of the arguments.The reason I didn't use
AnyIntas of now is because the input argumentsMgo through a private function_len_guardswhich don't allownp.bool_as a valid input argument type I believe, as in it just crashes if passed such an arg.