Skip to content

Check type annotations with pyright in CI - #21

Merged
roytsmart merged 3 commits into
mainfrom
fix-type-annotations
Aug 20, 2026
Merged

Check type annotations with pyright in CI#21
roytsmart merged 3 commits into
mainfrom
fix-type-annotations

Conversation

@roytsmart

@roytsmart roytsmart commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Adds a typing workflow that runs pyright, and fixes the annotations it complained about. This matters because #14 started shipping a py.typed marker, which tells downstream type checkers that these annotations are authoritative. named-arrays, optika, and esis all 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_min and spd_max were annotated None | np.ndarray, which rejects the plain numbers they clearly accept. test_colorbar_duplicate_wavelength already calls colorbar(spd_min=0.0, spd_max=1.0). They are now None | float | np.ndarray.
  • rgb_and_colorbar required wavelength, even though it forwards to rgb and colorbar, which both accept :obj:None and 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, and color_matching_xyz were declared to return astropy.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_illuminant genuinely does return a Quantity, with units of reciprocal wavelength, but numpy.interp is declared to return a plain array, so this one needs a cast with a comment saying why.
  • wavelength_visible_min and wavelength_visible_max are now declared as Quantity. Without that, 380 * u.nm infers 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:

  • rgb and colorbar duplicated the logic that builds a default wavelength grid, and assigned a plain array to a parameter declared None | u.Quantity. That is now a shared _wavelength_normalized helper which returns a Quantity, so the type is consistent from the first line and the duplication is gone. rgb_and_colorbar uses it too, which also removes a None check.
  • The closures returned by _transform_wavelength and _transform_spd_wavelength rebound their own parameters to different types. They now use separate names and declare their return types, so _transform_spd_wavelength advertises that it returns a wavelength in physical units.
  • rgb, colorbar, and XYZcie1931_from_spd rebound spd and wavelength to 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_normalize rebound a list to a tuple, and rgb_and_colorbar passed its arguments through an untyped dict, which silently defeated argument checking on both calls it made. Both are fixed.

Seven cast calls remain, each at a genuine gap in a dependency's stubs.

The output is unchanged. I compared rgb, colorbar, and rgb_and_colorbar against main, including both wavelength=None paths 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.typed marker, 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 in pyproject.toml configures 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.nm are inferred as a union of unit types rather than a Quantity, 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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (548d747) to head (ee20864).

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     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

roytsmart and others added 2 commits August 19, 2026 17:37
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@roytsmart roytsmart changed the title Check type annotations with mypy in CI Check type annotations with pyright in CI Aug 19, 2026
@roytsmart
roytsmart merged commit c94355b into main Aug 20, 2026
17 checks passed
@roytsmart
roytsmart deleted the fix-type-annotations branch August 20, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant