Uh oh!
There was an error while loading. Please reload this page.
Enable type-checking for random_objects.py - #369
Conversation
This commit adds type annotations to `random_objects.py`, along with its corresponding tests, enabling full type-checking with mypy while preserving existing functionality. The exclusion of `device_interface.py` is removed from `pyproject.toml` since this module was removed in TeamGraphix#261. `scipy-stubs` is added to `requirements-dev.txt` since `scipy` is used in `random_objects.py`. **Related issue:** This PR continues the work started in TeamGraphix#302, TeamGraphix#308, TeamGraphix#312 and TeamGraphix#347.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## master #369 +/- ##
==========================================
- Coverage 86.25% 86.25% -0.01%
==========================================
Files 44 44 Lines 6163 6161 -2 ==========================================
- Hits 5316 5314 -2
Misses 847 847 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| def rand_unit(sz: int, rng: Generator | None = None) -> npt.NDArray[np.complex128]: | ||
| def rand_unit(sz: IntLike, rng: Generator | None = None) -> npt.NDArray[np.float64]: |
There was a problem hiding this comment.
Why np.float64 instead of np.complex128 ?
There was a problem hiding this comment.
I added this comment in ffe24c2:
# unitary_group.rvs returns onp.Array3D[np.float64]# https://github.com/scipy/scipy-stubs/blob/3b629159e8da5cc3aa82b871135489d6d2fd5f8e/scipy-stubs/stats/_multivariate.pyi#L370There was a problem hiding this comment.
I think this is a mistake in the stubs:
importnumpyasnpfromgraphix.random_objectsimportrand_unitassertrand_unit(3).dtype==np.complex128Also, rand_unit(sz=1) clearly returns a complex number.
There was a problem hiding this comment.
Oops, yes, you're right of course! I reported the issue upstream (scipy/scipy-stubs#987) and I temporarily fixed the annotation in 202788b by coercing the result with .astype(np.complex128, copy=False) (which should be a no-op, since the array at runtime is already np.complex128).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
matulni
left a comment
There was a problem hiding this comment.
Small comment on the bug-fix, otherwise looks good to go!
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: matulni <m.uldemolins@gmail.com>
emlynsg
left a comment
There was a problem hiding this comment.
Minor comment, but shouldn't hold up merge.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This commit adds type annotations to `random_objects.py`, along with its corresponding tests, enabling full type-checking with mypy while preserving existing functionality. The exclusion of `device_interface.py` is removed from `pyproject.toml` since this module was removed in #261. `scipy-stubs` is added to `requirements-dev.txt` since `scipy` is used in `random_objects.py`. Revealed bad type annotation for `unitary_group.rvs`: scipy/scipy-stubs#987 **Related issue:** This PR continues the work started in #302, #308, #312 and #347. --------- Co-authored-by: matulni <m.uldemolins@gmail.com>
This commit adds type annotations to
random_objects.py, along with its corresponding tests, enabling full type-checking with mypy while preserving existing functionality.The exclusion of
device_interface.pyis removed frompyproject.tomlsince this module was removed in #261.scipy-stubsis added torequirements-dev.txtsincescipyis used inrandom_objects.py.