Skip to content

Fix: Adding Pace and PySHiELD unit tests back in to workflow #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fmalatino merged 14 commits into from
Jul 22, 2025
Merged

Fix: Adding Pace and PySHiELD unit tests back in to workflow #66

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5df509d
Adding back in pyshield tests
fmalatino Jul 15, 2025
d6b8241
New linting tools
fmalatino Jul 15, 2025
4c95d9e
Activating Pace unit tests
fmalatino Jul 15, 2025
0e4a18c
Removing setup.cfg
fmalatino Jul 15, 2025
bb71cb8
Fixing translate test workflow
fmalatino Jul 15, 2025
96a7e02
Merging in develop
fmalatino Jul 16, 2025
4c60be7
Adding Flake8-pyproject as additional dependency in pre-commit-config…
fmalatino Jul 16, 2025
6681412
Removing bumpversion tool
fmalatino Jul 17, 2025
63c69aa
Removing bdist_wheel universal 1 and using per-file-ignore for __init…
fmalatino Jul 17, 2025
5177a0c
Fixed config file for mypy in .pre-commit-config.yaml
fmalatino Jul 21, 2025
f2e050f
Using __all__ to avoid unused import error in __init__ files
fmalatino Jul 21, 2025
31779a3
Changing exclude in pyproject.toml to use correct case for directory …
fmalatino Jul 22, 2025
a420c82
Merge branch 'develop' into workflow_fix
fmalatino Jul 22, 2025
bf51aab
Linting
fmalatino Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions .github/workflows/lint.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@ jobs:
lint:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Checkout repository
uses: actions/checkout@v4

- name: Setup Python 3.11
uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Setup Python 3.11
uses: actions/setup-python@v5
with:
python-version: '3.11'

- name: Install pre-commit
run: |
pip install pre-commit
- name: Run lint via pre-commit
run: |
pre-commit run --all-files
- name: Install pre-commit
run: |
pip install pre-commit
- name: Run lint via pre-commit
run: |
pre-commit run --all-files

- name: Check advection code consistency
run: |
./.github/workflows/scripts/ensure_xppm_yppm_consistency.sh
- name: Check advection code consistency
run: |
./.github/workflows/scripts/ensure_xppm_yppm_consistency.sh
23 changes: 9 additions & 14 deletions .github/workflows/pace_tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
# TODO: Re-activate after PR 60
# TODO: Temporarily removing tests due to dependent changes in
# Pace#129, PyFV3#64 PRs; tests should be reverted back after
# these PRs are merged.
name: "pace main tests"
on:
pull_request:

# name: "pace main tests"
# on:
# pull_request:

# jobs:
# pace_main_tests:
# uses: NOAA-GFDL/pace/.github/workflows/main_unit_tests.yaml@develop
# with:
# component_trigger: true
# component_name: pyFV3
jobs:
pace_main_tests:
uses: NOAA-GFDL/pace/.github/workflows/main_unit_tests.yaml@develop
with:
component_trigger: true
component_name: pyFV3
19 changes: 9 additions & 10 deletions .github/workflows/pyshield_tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
# TODO: Reactivate after PR 60
# name: "pySHiELD translate tests"
# on:
# pull_request:
name: "pySHiELD translate tests"
on:
pull_request:

# jobs:
# pyshield_translate_tests:
# uses: NOAA-GFDL/PySHiELD/.github/workflows/translate.yaml@develop
# with:
# component_trigger: true
# component_name: pyFV3
jobs:
pyshield_translate_tests:
uses: NOAA-GFDL/PySHiELD/.github/workflows/translate.yaml@develop
with:
component_trigger: true
component_name: pyFV3
26 changes: 13 additions & 13 deletions .github/workflows/translate.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
uses: actions/checkout@v4
with:
submodules: 'recursive'
repository: noaa-gfdl/pyFV3
repository: noaa-gfdl/pyFV3
path: pyFV3

- name: Checkout hash that triggered CI
Expand All @@ -43,14 +43,14 @@ jobs:

- name: External trigger install NDSL packages
if: ${{inputs.component_trigger}}
run: |
run: |
cd ${GITHUB_WORKSPACE}/pyFV3
cd NDSL && pip3 install .[test] && cd ../
pip3 install .[test]

- name: Install pyFV3 packages
if: ${{ ! inputs.component_trigger }}
run : |
run: |
cd ${GITHUB_WORKSPACE}/pyFV3
pip install .[ndsl,test]

Expand All @@ -73,7 +73,7 @@ jobs:
- name: NumPy FvTp2d
run: |
cd ${GITHUB_WORKSPACE}/pyFV3
coverage run --rcfile=setup.cfg -m pytest \
coverage run --rcfile=pyproject.toml -m pytest \
-v -s --data_path=${{ env.DATA_PATH }} \
--backend=numpy \
--which_modules=FvTp2d \
Expand All @@ -82,18 +82,18 @@ jobs:

- name: Numpy D_SW
run: |
cd ${GITHUB_WORKSPACE}/pyFV3
coverage run --rcfile=setup.cfg -m pytest \
-v -s --data_path=${{ env.DATA_PATH }} \
--backend=numpy \
--which_modules=D_SW \
--threshold_overrides_file=./tests/savepoint/translate/overrides/standard.yaml \
./tests/savepoint
cd ${GITHUB_WORKSPACE}/pyFV3
coverage run --rcfile=pyproject.toml -m pytest \
-v -s --data_path=${{ env.DATA_PATH }} \
--backend=numpy \
--which_modules=D_SW \
--threshold_overrides_file=./tests/savepoint/translate/overrides/standard.yaml \
./tests/savepoint

- name: Numpy Remapping
run: |
cd ${GITHUB_WORKSPACE}/pyFV3
coverage run --rcfile=setup.cfg -m pytest \
coverage run --rcfile=pyproject.toml -m pytest \
-v -s --data_path=${{ env.DATA_PATH }} \
--backend=numpy \
--which_modules=Remapping \
Expand All @@ -107,7 +107,7 @@ jobs:
export PACE_FLOAT_PRECISION=64
export OMP_NUM_THREADS=1
export PACE_LOGLEVEL=Debug
mpiexec -mca orte_abort_on_non_zero_status 1 -np 6 --oversubscribe coverage run --rcfile=setup.cfg -m mpi4py -m pytest \
mpiexec -mca orte_abort_on_non_zero_status 1 -np 6 --oversubscribe coverage run --rcfile=pyproject.toml -m mpi4py -m pytest \
-v -s --data_path=${{ env.DATA_PATH }} \
--backend=dace:cpu \
-m parallel \
Expand Down
78 changes: 43 additions & 35 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,51 +2,59 @@ default_language_version:
python: python3

repos:
- repo: https://github.com/psf/black
- repo: https://github.com/psf/black
rev: 25.1.0
hooks:
- id: black
additional_dependencies: ["click==8.0.4"]
- id: black
additional_dependencies: ["click==8.0.4"]
Comment thread
romanc marked this conversation as resolved.
Outdated

- repo: https://github.com/pre-commit/mirrors-isort
- repo: https://github.com/pre-commit/mirrors-isort
rev: v5.10.1
hooks:
- id: isort
args: ["--profile", "black"]
- id: isort
args: ["--profile", "black"]

- repo: https://github.com/pre-commit/mirrors-mypy
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.17.0
hooks:
- id: mypy
name: mypy-fv3core
args: [--config-file, setup.cfg]
additional_dependencies: [types-PyYAML]
files: pyFV3
exclude: |
(?x)^(
pyFV3/stencils/fv_subgridz.py |
pyFV3/tests/conftest.py
)$
- repo: https://github.com/pre-commit/pre-commit-hooks
- id: mypy
name: mypy-fv3core
args: [--config-file, setup.cfg]
additional_dependencies: [types-PyYAML]
Comment thread
romanc marked this conversation as resolved.
Outdated
files: pyFV3
exclude: |
(?x)^(
pyFV3/stencils/fv_subgridz.py |
pyFV3/tests/conftest.py
)$
Comment thread
romanc marked this conversation as resolved.
Outdated
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: check-toml
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/pycqa/flake8
- id: check-toml
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://github.com/pycqa/flake8
rev: 7.3.0
hooks:
- id: flake8
name: flake8
language_version: python3
args: [--config, setup.cfg]
exclude: |
(?x)^(
.*/__init__.py |
)$
- id: flake8
name: flake8 __init__.py files
files: "__init__.py"
- id: flake8
name: flake8
language_version: python3
additional_dependencies: [Flake8-pyproject]
exclude: |
(?x)^(
.*/__init__.py |
)$
- id: flake8
name: flake8 __init__.py files
additional_dependencies: [Flake8-pyproject]
files: "__init__.py"
# ignore unused import error in __init__.py files
Comment thread
romanc marked this conversation as resolved.
Outdated
args: ["--ignore=F401,E203", --config, setup.cfg]

- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
rev: v2.15.0
hooks:
- id: pretty-format-toml
args: [--autofix, --indent, "2"]
- id: pretty-format-yaml
args: [--autofix, --preserve-quotes, --indent, "2", --offset, "2"]
111 changes: 76 additions & 35 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,98 @@
[build-system]
requires = ["setuptools >= 77.0.3"]
build-backend = "setuptools.build_meta"
requires = ["setuptools >= 77.0.3"]

[project]
name = "pyfv3"
version = "0.2.0"
authors = [{name = "NOAA - Geophysical Fluid Dynamics Laboratory", email = "oliver.elbert@noaa.gov"}]
classifiers = [
"Development Status :: 2 - Pre-Alpha",
"Intended Audience :: Developers",
"Natural Language :: English",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.11"
]
dependencies = [
"f90nml>=1.1.0",
"numpy==1.26.4",
"xarray",
"f90nml>=1.1.0",
"numpy==1.26.4",
"xarray"
]
requires-python = ">=3.11,<3.12"
authors = [{name = "NOAA - Geophysical Fluid Dynamics Laboratory", email = "oliver.elbert@noaa.gov"}]
description = "PyFV3 is a NDSL-based FV3 dynamical core for atmospheric models."
readme = "README.md"
license = "Apache-2.0"
license-files = ["LICENSE.md"]
classifiers = [
"Development Status :: 2 - Pre-Alpha",
"Intended Audience :: Developers",
"Natural Language :: English",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.11",
]
name = "pyfv3"
readme = "README.md"
requires-python = ">=3.11,<3.12"
version = "0.2.0"

[project.optional-dependencies]
test = [
"coverage",
"pytest",
"pytest-subtests",
"serialbox",
]
ndsl = ["ndsl @ git+https://github.com/NOAA-GFDL/NDSL.git@develop"]
develop = [
"pyfv3[test]",
"pyfv3[ndsl]",
"pre-commit",
dev = [
"pyfv3[test]",
"pyfv3[ndsl]",
"pre-commit",
"flake8-pyproject"
]
extras = [
"pyfv3[test]",
"pyfv3[ndsl]",
"pyfv3[develop]",
"pyfv3[test]",
"pyfv3[ndsl]",
"pyfv3[dev]"
]
ndsl = ["ndsl @ git+https://github.com/NOAA-GFDL/NDSL.git@develop"]
test = [
"coverage",
"pytest",
"pytest-subtests",
"serialbox"
]

[project.urls]
Repository = "https://github.com/NOAA-GFDL/pyFV3"

[tool.setuptools.packages.find]
include = ["pyfv3", "pyfv3.*"]

[tool.setuptools]
include-package-data = true
[tool.aliases]

[tool.black]
line-length = 88
target_version = ['py311']

[tool.bumpversion]
commit = "True"
current_version = "0.2.0"
Comment thread
romanc marked this conversation as resolved.
Outdated

[tool.coverage.run]
branch = true
omit = ["tests/*", "*gt_cache*", ".dacecache*", "external/*", "__init__.py"]
parallel = true
source_pkgs = ["pyfv3"]

[tool.disutils.bdist_wheel]
universal = 1
Comment thread
romanc marked this conversation as resolved.
Outdated

[tool.flake8]
exclude = ["docs"]
extend-ignore = ["W503", "E302", "E203", "F841", "F401"]

@romanc romanc Jul 17, 2025

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.

This adds the exception for F401 (no unused imports) to all files (not just __init__.py files as before). I think it would be good if we pruned unused imports in all files other than __init__.py (which can have convenience imports/exports).

@FlorianDeconinck FlorianDeconinck Jul 17, 2025

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.

In the __init__.py you can do

import .moduleA
import .moduleB

__all__ = ["moduleA", "moduleB"]

@romanc romanc Jul 17, 2025

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.

Yes, this is why the F401 exception should be there for __init__.py files. I'm arguing it should only be there for __init__.py files (and not for any other python files).

@FlorianDeconinck FlorianDeconinck Jul 17, 2025

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.

But if you do the __all__, which is the spec, I don't think you need the F401 at all (I could be wrong)

@romanc romanc Jul 18, 2025

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.

Ah okay, today I learned ... Good to know!

max-line-length = 88

[tool.isort]
default_section = "THIRDPARTY"
force_grid_wrap = 0
include_trailing_comma = true
known_first_party = "pyfv3,ndsl,pyshield"
known_third_party = "f90nml,pytest,xarray,numpy,mpi4py,gt4py"
line_length = 88
lines_after_imports = 2
multi_line_output = 3
sections = "FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCALFOLDER"
use_parentheses = true

[tool.mypy]
explicit_package_bases = true
follow_imports = "normal"
ignore_missing_imports = true
namespace_packages = true
strict_optional = false
warn_unreachable = true

[tool.setuptools]
include-package-data = true

[tool.setuptools.packages.find]
include = ["pyfv3", "pyfv3.*"]
Loading