Check type annotations with pyright in CI - #21
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 452 450 -2
=========================================
- Hits 452 450 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Adds a
typingworkflow that runs pyright, and fixes the annotations it complained about. This matters because #14 started shipping apy.typedmarker, which tells downstream type checkers that these annotations are authoritative.named-arrays,optika, andesisall consume this package, so a wrong annotation here propagates into their inference. Until now nothing verified them.Pyright reports no errors and no warnings on the package. Getting there took the count from 48 down through 30, 14, 10, and 3 to zero.
Two real bugs in the public API
spd_minandspd_maxwere annotatedNone | np.ndarray, which rejects the plain numbers they clearly accept.test_colorbar_duplicate_wavelengthalready callscolorbar(spd_min=0.0, spd_max=1.0). They are nowNone | float | np.ndarray.rgb_and_colorbarrequiredwavelength, even though it forwards torgbandcolorbar, which both accept :obj:Noneand build a default grid. It is now optional, with the same default as those two.Annotation fixes
color_matching_x,color_matching_y,color_matching_z, andcolor_matching_xyzwere declared to returnastropy.units.Quantity, but the color matching functions are dimensionless and these have always returned a plain array. I confirmed the runtime types before changing the annotations.d65_standard_illuminantgenuinely does return aQuantity, with units of reciprocal wavelength, butnumpy.interpis declared to return a plain array, so this one needs acastwith a comment saying why.wavelength_visible_minandwavelength_visible_maxare now declared asQuantity. Without that,380 * u.nminfers as a union of five different unit classes, and subtracting them was an error.Restructuring, which is most of the fix
Rather than casting around the problems, the cases where a variable changed type partway through a function were rewritten:
rgbandcolorbarduplicated the logic that builds a default wavelength grid, and assigned a plain array to a parameter declaredNone | u.Quantity. That is now a shared_wavelength_normalizedhelper which returns aQuantity, so the type is consistent from the first line and the duplication is gone.rgb_and_colorbaruses it too, which also removes aNonecheck._transform_wavelengthand_transform_spd_wavelengthrebound their own parameters to different types. They now use separate names and declare their return types, so_transform_spd_wavelengthadvertises that it returns a wavelength in physical units.rgb,colorbar, andXYZcie1931_from_spdreboundspdandwavelengthto transformed values. Those now use distinct local names, which also makes it easier to follow which stage of the pipeline a value belongs to._bounds_normalizerebound alistto atuple, andrgb_and_colorbarpassed its arguments through an untypeddict, which silently defeated argument checking on both calls it made. Both are fixed.Seven
castcalls remain, each at a genuine gap in a dependency's stubs.The output is unchanged. I compared
rgb,colorbar, andrgb_and_colorbaragainstmain, including bothwavelength=Nonepaths and a call with explicit bounds, and every checksum matches to all printed digits. All 79 tests pass.Why pyright, and why not mypy
The two disagree sharply here, and the reason is worth recording: astropy does not ship a
py.typedmarker, while numpy does. Mypy honors that, per PEP 561, and treats astropy as untyped, so it reported only three genuine problems and was silent about everything else, including both of the API bugs above. Pyright infers from astropy's source instead, which is noisier but found real defects.Pyright is also what Pylance runs, so a single
[tool.pyright]section inpyproject.tomlconfigures the editor and CI together, which mypy cannot do.The tests are excluded from the check, with the reason recorded in the configuration. Expressions like
numpy.linspace(...) * astropy.units.nmare inferred as a union of unit types rather than aQuantity, and the tests are built almost entirely from those. Silencing it required a cast on nearly every fixture, which made the tests materially harder to read for no safety benefit.Pyright is pinned to 1.1.411 so that a new release cannot fail CI on an unrelated pull request. Note that Pylance bundles its own pyright, so an out-of-date extension can still disagree with CI.
🤖 Generated with Claude Code