fortran bindings: split py2fgen and bindings#1185
Merged
Conversation
egparedes
approved these changes
Mar 31, 2026
Contributor
egparedes
left a comment
egparedes
left a comment
There was a problem hiding this comment.
A couple of comments but in general LGTM
Comment on lines
+88
to
+90
| yield click_testing.CliRunner() | ||
| os.environ["FLOAT_PRECISION"] = type_alias.DEFAULT_PRECISION | ||
| type_alias.set_precision(type_alias.DEFAULT_PRECISION) |
Contributor
There was a problem hiding this comment.
I'm not completely sure if I understand this correctly, but if the idea is to restore default settings after testing the CLI, shouldn't the actual original values be saved beforehand and restored later?
Suggested change
| yield click_testing.CliRunner() | |
| os.environ["FLOAT_PRECISION"] = type_alias.DEFAULT_PRECISION | |
| type_alias.set_precision(type_alias.DEFAULT_PRECISION) | |
| old_env = os.environ["FLOAT_PRECISION"] | |
| old_precision_alias = type_alias.precision | |
| yield click_testing.CliRunner() | |
| os.environ["FLOAT_PRECISION"] = old_env | |
| type_alias.set_precision(old_precision_alias) |
Contributor
Author
There was a problem hiding this comment.
it was just copied over from tools. delete now as it's unused I think.
jcanton
requested changes
Mar 31, 2026
Contributor
jcanton
left a comment
jcanton
left a comment
There was a problem hiding this comment.
looks good other than one/two small edits:
maybe better to move bindings in model/ to better separate?
Contributor
There was a problem hiding this comment.
Pull request overview
This PR separates the Fortran wrapper modules (previously living under icon4py.tools.py2fgen) into a new workspace package, icon4py.bindings, so that py2fgen remains a standalone tool while bindings become a dedicated, model-dependent package.
Changes:
- Introduce the new
icon4py-bindingspackage (source + tests) and update imports fromicon4py.tools.py2fgen.wrappers.*toicon4py.bindings.*. - Update workspace/configuration (uv workspace, extras, mypy/pytest paths, tach modules) and CI to run bindings + tool tests.
- Move/refresh codegen reference artifacts under
bindings/tests/bindings/references/.
Reviewed changes
Copilot reviewed 29 out of 43 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Adds icon4py-bindings to workspace/extras; adjusts deps |
tools/tests/tools/py2fgen/wrappers/__init__.py |
Removes old wrappers test package init |
tools/tests/tools/py2fgen/test_generator.py |
Updates wrapper module path to icon4py.bindings.* |
tools/tests/tools/py2fgen/test_cli.py |
Uses icon4py.bindings.simple + new plugin path |
tools/tests/tools/conftest.py |
Removes previously shared fixtures/hooks |
tools/pyproject.toml |
Drops model deps; adjusts ruff excludes |
tach.toml |
Adds bindings/src; updates module graph |
pyproject.toml |
Adds bindings to workspace, mypy/pytest paths, and fortran extra |
noxfile.py |
Reworks tools session into tools+bindings session |
model/standalone_driver/pyproject.toml |
Drops pytest from runtime deps |
bindings/tests/conftest.py |
New bindings-level conftest with fixtures + pytest hooks |
bindings/tests/bindings/utils.py |
Adds test comparison helpers |
bindings/tests/bindings/test_icon4py_export.py |
Updates export import to bindings |
bindings/tests/bindings/test_grid_init.py |
Updates wrapper imports to bindings |
bindings/tests/bindings/test_dycore_wrapper.py |
Updates wrapper imports to bindings |
bindings/tests/bindings/test_diffusion_wrapper.py |
Updates wrapper imports to bindings |
bindings/tests/bindings/test_codegen_references.py |
Updates reference module paths to bindings |
bindings/tests/bindings/references/grid/grid.py |
Updates embedded imports to bindings |
bindings/tests/bindings/references/grid/grid.h |
New generated C header reference |
bindings/tests/bindings/references/grid/grid.f90 |
New generated Fortran module reference |
bindings/tests/bindings/references/dycore/dycore.py |
Updates embedded imports to bindings |
bindings/tests/bindings/references/dycore/dycore.h |
New generated C header reference |
bindings/tests/bindings/references/dycore/dycore.f90 |
New generated Fortran module reference |
bindings/tests/bindings/references/diffusion/diffusion.py |
Updates embedded imports to bindings |
bindings/tests/bindings/references/diffusion/diffusion.h |
New generated C header reference |
bindings/tests/bindings/references/diffusion/diffusion.f90 |
New generated Fortran module reference |
bindings/tests/bindings/.gitignore |
Ignores compiled reference artifacts |
bindings/tests/bindings/__init__.py |
Adds test package init/header |
bindings/src/icon4py/bindings/viztracer_plugin.py |
Moves plugin to bindings; updates imports |
bindings/src/icon4py/bindings/simple.py |
Uses bindings export hook |
bindings/src/icon4py/bindings/muphys_wrapper.py |
Moves wrapper to bindings; tweaks typing comment |
bindings/src/icon4py/bindings/icon4py_export.py |
New bindings export hooks for GT4Py annotations |
bindings/src/icon4py/bindings/grid_wrapper.py |
Moves grid wrapper to bindings package |
bindings/src/icon4py/bindings/dycore_wrapper.py |
Moves dycore wrapper to bindings package |
bindings/src/icon4py/bindings/diffusion_wrapper.py |
Moves diffusion wrapper to bindings package |
bindings/src/icon4py/bindings/debug_utils.py |
New debug logging utilities |
bindings/src/icon4py/bindings/config.py |
Adds env-driven config flag |
bindings/src/icon4py/bindings/common.py |
New shared helpers/types for bindings |
bindings/src/icon4py/bindings/all_bindings.py |
Aggregates exported binding functions |
bindings/src/icon4py/bindings/__init__.py |
New bindings package metadata |
bindings/pyproject.toml |
Defines new icon4py-bindings package |
.pre-commit-config.yaml |
Updates excludes to new references location |
.github/workflows/icon4py-test-tools-and-bindings.yml |
CI runs updated nox session for tools+bindings |
Comments suppressed due to low confidence (1)
tools/tests/tools/conftest.py:17
- The nox session runs
pytest ... --datatest-only/--datatest-skipin thetools/directory, but thisconftest.pyno longer loadsicon4py.model.testing.pytest_hooksthat define these custom CLI options. As a result, pytest will error with “unrecognized arguments” when these flags are used. Either re-add the hook import here (as before) or stop passing the datatest flags when running thetoolstest suite.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
cscs-ci run default |
Contributor
Author
|
cscs-ci run distributed |
Contributor
Author
|
cscs-ci run default |
Contributor
Author
|
cscs-ci run default |
|
Mandatory Tests Please make sure you run these tests via comment before you merge!
Optional Tests To run benchmarks you can use:
To run tests and benchmarks with the DaCe backend you can use:
To run test levels ignored by the default test suite (mostly simple datatest for static fields computations) you can use:
For more detailed information please look at CI in the EXCLAIM universe. |
Contributor
Author
|
cscs-ci run distributed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The actual Fortran bindings of diffusion and dycore where part of the tools/py2fgen package. However py2fgen is actually a standalone tool.
We introduce a new package
icon4py.bindingswhich depends on py2fgen and the atmosphere packages that it's generating bindings for.Longer term it might be better to make the bindings part of their respective packages as optionals.