add stub files for Cython modules in pywt/_extensions#847
add stub files for Cython modules in pywt/_extensions#847NiklasMueller2001 wants to merge 2 commits intoPyWavelets:mainfrom
Conversation
jorenham
left a comment
jorenham
left a comment
There was a problem hiding this comment.
Yay; stubs 🎉
I took a look and left some comments; hope you don't mind :)
| "cmor", | ||
| ] | ||
|
|
||
| DataT = TypeVar("DataT", np.float32, np.float64) |
There was a problem hiding this comment.
It's usually best to avoid using type variable constraints like this, and instead use upper bounds.
One reason for that is that there's quite a lot of difference in how different type-checkers deal with constraints, because it's under-specified in the typing spec.
Another is that typevars with constraints don't accept typevars with bounds, which can be annoying for downstream library maintainers as they'd be forced to also use constraints. The other way around is no problem.
| DataT = TypeVar("DataT", np.float32, np.float64) | |
| DataT = TypeVar("DataT", bound=np.float32 | np.float64) |
There was a problem hiding this comment.
Thanks for this tip, I was not aware of that issue
| CDataT = TypeVar( | ||
| "CDataT", | ||
| np.float32, | ||
| np.float64, | ||
| np.complex64, | ||
| np.complex128, | ||
| ) |
There was a problem hiding this comment.
| CDataT = TypeVar( | |
| "CDataT", | |
| np.float32, | |
| np.float64, | |
| np.complex64, | |
| np.complex128, | |
| ) | |
| CDataT = TypeVar( | |
| "CDataT", | |
| bound=np.float32 | np.float64 | np.complex64 | np.complex128, | |
| ) |
| np.complex128, | ||
| ) | ||
|
|
||
| _Kind = Literal["all", "continuous", "discrete"] |
There was a problem hiding this comment.
Type-checkers like it if you help them a bit and explicitly mark it as type alias:
| _Kind = Literal["all", "continuous", "discrete"] | |
| _Kind: TypeAlias = Literal["all", "continuous", "discrete"] |
(you'll have to import TypeAlias from typing though)
| def swt(data: NDArray[CDataT], wavelet: Wavelet, level: int, start_level: int, trim_approx: bool = ...) -> NDArray[CDataT]: ... | ||
| def swt_axis(data: NDArray[CDataT], wavelet: Wavelet, level: int, start_level: int, axis: int = ..., trim_approx: bool = ...) -> NDArray[CDataT]: ... |
There was a problem hiding this comment.
For documentation purposes it can help to fill in the ... defaults. The typing spec stub style guide also recommends this: https://typing.python.org/en/latest/guides/writing_stubs.html#functions-and-methods
| def dwt_coeff_len(size_t: int, filter_len: int, mode: MODE) -> int: ... | ||
| def dwt_single(data: NDArray[CDataT], wavelet: Wavelet, mode: MODE) -> tuple[NDArray[CDataT], NDArray[CDataT]]: ... | ||
| def dwt_axis(data: NDArray[CDataT], wavelet: Wavelet, mode: MODE, axis: int = ...) -> tuple[NDArray[CDataT], NDArray[CDataT]]: ... | ||
| def idwt_single(cA: NDArray[CDataT], cD: NDArray[CDataT], wavelet: Wavelet, mode: MODE) -> NDArray[CDataT]: ... | ||
| def idwt_axis(coefs_a: NDArray[CDataT], coefs_d: NDArray[CDataT], wavelet: Wavelet, mode: MODE, axis: int = ...) -> NDArray[CDataT]: ... | ||
| def upcoef(do_rec_a: bool, coeffs: NDArray[CDataT], wavelet: Wavelet, level: int, take: int) -> NDArray[CDataT]: ... | ||
| def downcoef(do_dec_a: bool, data: NDArray[CDataT], wavelet: Wavelet, mode: MODE, level: int) -> NDArray[CDataT]: ... |
There was a problem hiding this comment.
mode: MODE won't accept bare integers now, so type-checkers will now complain if you pass mode=1. You could use a Literal instead of an IntEnum to declare MODE if that's now what you want.
There was a problem hiding this comment.
Thanks for the suggestion. I would ideally want to accept both the enum keys and valid integers, so something like this could solve it:
class MODE(IntEnum):
MODE_INVALID = -1
MODE_ZEROPAD = 0
MODE_SYMMETRIC = 1
MODE_CONSTANT_EDGE = 2
MODE_SMOOTH = 3
MODE_PERIODIC = 4
MODE_PERIODIZATION = 5
MODE_REFLECT = 6
MODE_ANTISYMMETRIC = 7
MODE_ANTIREFLECT = 8
MODE_MAX = 9
ModeInt = MODE | Literal[-1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]Unfortunately there seems to be no better way of accepting only the integer values that the enum maps to, but I think the MODE class should be stable enough to allow this small duplication.
There was a problem hiding this comment.
That looks like a good solution.
but I think the
MODEclass should be stable enough to allow this small duplication.
In Python typing land code duplication is unavoidable I'm afraid...
- Added default values in stub files. - Added ModeInt type union to allow integer values where MODE enum was expected. - Used upper bounds for DataT and CDataT types instead of variable constraints.
3 participants
This PR aims to implement typing support for public functions and classes that are implemented in
Cythonmodules insidepywt/_extensionsby adding stub files.Changes
pywt/__init__.pyto help type checkers to infer correct types..pyxinpywt/_extensions.