Skip to content

Conversation

@romanc
Copy link
Collaborator

@romanc romanc commented Jan 14, 2026

Description

⚠️ Work in progress ⚠️

This PR re-enables mypy (not only for the source code directory) in pyshield. It then fixes the obvious issues and leaves two things to discuss:

  1. In PBLConfig, should dt_atmos be of type int (as type-hinted now) or is it supposed to be a float (like other usage of dt_atmos would suggest?
  2. translate_pbl_subtests is claiming that pyshield.constants contains a constant called CDTN, where such a constant doesn't exist. Should that one be imported from pyshield.stencils.pbl.constants?

@oelbert I guess you are the expert here. What do you think about the question above?

This PR is stacked on top of #81 and should be merged afterwards. This also explains the pyshield-side of failures in NOAA-GFDL/pace#173 and why we haven't seen them before.

How Has This Been Tested?

All good once CI is green.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
    See inline comments
  • I have made corresponding changes to the documentation: N/A
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules: N/A
  • New check tests, if applicable, are included
  • Targeted model, if this change was triggered by a model need/shortcoming: N/A

- id: mypy
name: mypy-physics
args: ["--config-file", "pyproject.toml"]
files: pySHiELD
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is why mypy never complained because that's a case-sensitive file filter that wasn't updated when the source directory was renamed from pySHiELD to pyshield.

@romanc romanc force-pushed the romanc/re-enable-mypy branch from 5ee0f43 to 6fd7f10 Compare January 16, 2026 10:20
@romanc romanc requested a review from oelbert January 19, 2026 09:46
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