Skip to content

ci: stricter type checks with mypy#97

Merged
fmalatino merged 7 commits into
NOAA-GFDL:developfrom
romanc:romanc/pyfv3-types
Nov 20, 2025
Merged

ci: stricter type checks with mypy#97
fmalatino merged 7 commits into
NOAA-GFDL:developfrom
romanc:romanc/pyfv3-types

Conversation

@romanc
Copy link
Copy Markdown
Collaborator

@romanc romanc commented Oct 29, 2025 •
edited
Loading

Description

This PR suggests better defaults for the mypy configuration. The new rules apply don't apply within pyfv3.stencils. That would have been too much effort to start with. Types aren't less safe than before because before the exceptions were just made for the whole repo (now it's contained to pyfv3.stencils and select files that weren't typed before).

In addition, the PR includes

  • update of the linting tools
  • new tool: flake8-bugbear
  • more modern type hints (e.g. Dict[...] -> dict[...])

Note that even this configuration doesn't catch, what is about to be fixed in #96 (for NOAA-GFDL/pace#156).

/cc @twicki @FlorianDeconinck

How Has This Been Tested?

Self testing with new checks by mypy and flake8-bugbear. All good as long as CI (in particular the linting job) is still 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
  • 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 changed was triggered by a model need/shortcoming: N/A

@romanc romanc force-pushed the romanc/pyfv3-types branch 2 times, most recently from da9354e to ee439ac Compare October 29, 2025 13:39
@romanc
Copy link
Copy Markdown
Collaborator Author

romanc commented Oct 29, 2025

Sorry for the back and forth. This should be good for review now and is not time critical in any way.

@romanc romanc removed the request for review from FlorianDeconinck October 30, 2025 15:18
@romanc romanc force-pushed the romanc/pyfv3-types branch 2 times, most recently from bd863ca to ee581b0 Compare November 14, 2025 09:05
@romanc
remove unused ignore statements
1847679
@romanc
try to enforce types in pyfv3
8588543
@romanc
modern types in pyfv3
4b91e87
@romanc
Update versions of tools in pre-commit
3abd2d5
@romanc
enable flake8-bugbear
eb471df
@romanc
let's try with annotations from the future
ba707a3
@romanc
undo Optional["FloatField"] changes
a82f340 
apparently these break either with `|` not defined for
`_FieldDescriptor` or - in combination with  `from __future__ import
annotations` - we end up in GT4Py, which gets confused and can't
resolve the argument annotations anymore.
@romanc romanc force-pushed the romanc/pyfv3-types branch from ee581b0 to a82f340 Compare November 20, 2025 13:05
@romanc romanc requested review from FlorianDeconinck and removed request for jjuyeonkim and twicki November 20, 2025 13:06
@romanc
Copy link
Copy Markdown
Collaborator Author

romanc commented Nov 20, 2025

This PR has been stale for a while due to missing reviews ... If there are any concerns, please let me know. If not, I would be happy if this PR could merge soonish. Remember, I don't have the rights to merge in this repo ...

@jjuyeonkim
Copy link
Copy Markdown
Collaborator

jjuyeonkim commented Nov 20, 2025

This PR has been stale for a while due to missing reviews ... If there are any concerns, please let me know. If not, I would be happy if this PR could merge soonish. Remember, I don't have the rights to merge in this repo ...

My apologies. I don't know if I'm the one holding this up. I sent a message to mm just to clear up when it's okay to merge.

@fmalatino
Copy link
Copy Markdown
Contributor

fmalatino commented Nov 20, 2025

This PR has been stale for a while due to missing reviews ... If there are any concerns, please let me know. If not, I would be happy if this PR could merge soonish. Remember, I don't have the rights to merge in this repo ...

I was waiting on the review from @FlorianDeconinck, if he is ok with it, I will merge it.

FlorianDeconinck
FlorianDeconinck approved these changes Nov 20, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

Sorry folks, that fell off my plate, good to go !

@fmalatino fmalatino enabled auto-merge November 20, 2025 13:51
@fmalatino fmalatino added this pull request to the merge queue Nov 20, 2025
Merged via the queue into NOAA-GFDL:develop with commit 00181c7 Nov 20, 2025
5 checks passed
@romanc romanc deleted the romanc/pyfv3-types branch November 21, 2025 14:58
romanc pushed a commit to romanc/PyFV3 that referenced this pull request Feb 10, 2026
fix: DyCore orchestration without timer allocation
a491e42 
We can't do any allocation in DaCe code (for good reasons). This PR
fixes fallout from NOAA-GFDL#97 where we
accidentally introduced the allocaiton of `NullTimer()` in DaCe-owned
code. This PR moves the construction of the `Timer` up to calling code.

Unrelated, the PR reduces the indet level by adding early returns and
adds a couple of type hints here and there.
@romanc romanc mentioned this pull request Feb 10, 2026
Merged
8 tasks
romanc pushed a commit to romanc/PyFV3 that referenced this pull request Feb 11, 2026
fix: DyCore orchestration without timer allocation
8929dd7 
We can't do any allocation in DaCe code (for good reasons). This PR
fixes fallout from NOAA-GFDL#97 where we
accidentally introduced the allocaiton of `NullTimer()` in DaCe-owned
code. This PR moves the construction of the `Timer` up to calling code.

Unrelated, the PR reduces the indet level by adding early returns and
adds a couple of type hints here and there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FlorianDeconinck FlorianDeconinck FlorianDeconinck approved these changes

@fmalatino fmalatino fmalatino approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@romanc @jjuyeonkim @fmalatino @FlorianDeconinck