Skip to content

Type checking - #301

Draft
samaloney wants to merge 1 commit into
sunpy:mainfrom
samaloney:feature-typing
Draft

Type checking#301
samaloney wants to merge 1 commit into
sunpy:mainfrom
samaloney:feature-typing

Conversation

@samaloney

@samaloney samaloney commented Aug 9, 2026

Copy link
Copy Markdown
Member

PR Description

  • Add mypy and pyright type check configuration and CI job
  • Update code to pass new CI type check job
  • Review narrow/widen some types for Quantities etc just got it pass right now

If we're want to this it would be good to agree and merge sooner rather than later as touches nearly every file.

If we don't want the types inline could move them to stubs files

AI Assistance Disclosure

Help figure out some of the types and specific inline ignores and configuration

AI tools were used for:

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • [X ] Research and understanding
  • No AI tools were used

Regardless of AI use, the human contributor remains fully responsible for correctness, design choices, licensing compatibility, and long-term maintainability.

@KriSun95 KriSun95 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding types to be explicit is a great idea. I'd imagine this is something we'll continue to edit and adopt as we keep building too.

data_y: NDArray[np.float64],
model_x: NDArray[np.float64],
model_func: Any,
statistic_func: Callable[[NDArray[np.float64], NDArray[np.float64]], float],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work if the Callable can have keyword arguments other varying inputs?

Comment on lines +105 to +113
p: Any = p.default,
break_energy: Any = u.Quantity(break_energy.default, break_energy.unit), # pyright: ignore[reportArgumentType]
q: Any = q.default,
low_e_cutoff: Any = u.Quantity(low_e_cutoff.default, low_e_cutoff.unit), # pyright: ignore[reportArgumentType]
high_e_cutoff: Any = u.Quantity(high_e_cutoff.default, high_e_cutoff.unit), # pyright: ignore[reportArgumentType]
total_eflux: Any = u.Quantity(total_eflux.default, total_eflux.unit), # pyright: ignore[reportArgumentType]
integrator: Any = None,
**kwargs: Any,
) -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all of these in __init__ should be Astropy quantities even if dimensionless.

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.

2 participants