refactor: drop generated-code formatting from next, move formatter to eve.formatting by egparedes · Pull Request #2927 · GridTools/gt4py

egparedes · 2026-09-28T16:08:53Z

Description

Removes automatic formatting (black / clang-format) of generated code from gt4py.next and keeps it only for gt4py.cartesian (and tests), with the formatter reduced to a minimal best-effort module.

New leaf module gt4py.eve.formatting with format_python_source (black; line_length argument, default 100; black's target version pinned to the running interpreter) and format_cpp_source (clang-format, LLVM style); both return the input unchanged when the tool is missing or fails. The executable can still be overridden with CLANG_FORMAT_EXECUTABLE.
Removed the old formatting API from gt4py.eve.codegen (format_source, format_python_source, format_cpp_source, register_formatter, SOURCE_FORMATTERS, FormattingError, FormatterNameError). Importing eve.codegen no longer imports black nor spawns a clang-format subprocess.
gt4py.next: no formatting at all. SourceCodeSpec and subclasses lose formatter_key/formatter_options (specs are now hashable); otf.artifacts.format_source removed; gtfn sources, nanobind bindings and roundtrip debug output are emitted unformatted; formatters.gtfn.format_cpp returns raw C++. Roundtrip: debug removed from the source-cache key and added to the module-cache key (debug runs still write the temp .py file).
gt4py.cartesian: no behaviour change. It only switches its existing formatting calls from eve.codegen.format_source(...) to eve.formatting.format_cpp_source(...) / format_python_source(...); the call sites, the format_source option handling and the line lengths stay as they were.
tach.toml: gt4py.eve.formatting is its own module and only gt4py.cartesian may depend on it.
Dependencies: black is no longer a runtime dependency and clang-format moved out of the standard extra; both are now in the cartesian extra and the test dependency group. This is the only change visible from gt4py.cartesian: a plain pip install gt4py no longer formats cartesian output.
Supersedes feat[next]: add FORMAT_SOURCES config option to control formatting of generated code #2919.

Breaking changes / notes

Removed gt4py.eve.codegen formatting API; use gt4py.eve.formatting. Known downstream user: icon4py tools/src/icon4py/tools/py2fgen/_codegen.py (also needs black as its own dependency).
black/clang-format are no longer installed by default (pip install gt4py[cartesian] to get formatted cartesian output).
Generated gtfn C++ include order is no longer sorted by clang-format; CPU is covered by CI, GPU is covered only by CSCS CI.

Requirements

All fixes and/or new features come with corresponding tests. (New tests/eve_tests/unit_tests/test_formatting.py; existing next binding tests updated; new roundtrip module-cache test.)
Important design decisions have been documented in the appropriate ADR inside the docs/development/ADRs/ folder. (No new ADR needed; one-line note added to ADR 0012.)

If this PR contains code authored by new contributors please make sure:

The PR contains an updated version of the AUTHORS.md file adding the names of all the new contributors. (N/A)

… eve.formatting Remove automatic formatting (black / clang-format) of generated code from gt4py.next and keep it only for gt4py.cartesian (and tests). - New leaf module `gt4py.eve.formatting` with best-effort `format_python_source` (black) and `format_cpp_source` (clang-format); both return the input unchanged when the tool is missing or fails. - Remove the old formatting API from `gt4py.eve.codegen`. - gt4py.next: no formatting; `SourceCodeSpec` loses `formatter_key`/`formatter_options`; `otf.artifacts.format_source` removed. - gt4py.cartesian: C++ formatted once in `BaseGTBackend._make_extension_sources`; Python backends use `eve.formatting`. - tach: only gt4py.cartesian may depend on `gt4py.eve.formatting`. - Dependencies: black and clang-format moved to the `cartesian` extra and the `test` dependency group.

Copilot

Copilot review overview

🟡 Changes recommended

The breaking API removal lacks a changelog entry, and key cache and formatting behaviors lack focused regression tests.

Review effort: Balanced
Findings: 4

Open (4)

Cover format_source option for computation and binding sources · New
Test verbatim preservation of stencil-definition sources · New
Document removal of public codegen formatting API · New
Test module cache separation for debug mode · New

What changed in this PR

Moves generated-code formatting into an optional Eve utility, removes formatting from gt4py.next, and centralizes Cartesian formatting.

Changes:

Adds best-effort Python/C++ formatting with optional dependencies.
Emits raw Next code and updates roundtrip caching.
Preserves Cartesian formatting through a centralized backend pass.

File	Description
`uv.lock`	Updates formatter dependency groups.
`pyproject.toml`	Moves formatters to Cartesian/test extras.
`tach.toml`	Defines the formatting dependency boundary.
`src/gt4py/eve/formatting.py`	Adds best-effort formatter functions.
`src/gt4py/eve/codegen.py`	Removes the old formatting API.
`src/gt4py/eve/__init__.py`	Documents the new leaf module.
`src/gt4py/next/otf/artifacts.py`	Removes formatter metadata and helper.
`src/gt4py/next/otf/binding/nanobind.py`	Emits unformatted bindings.
`src/gt4py/next/program_processors/codegens/gtfn/gtfn_module.py`	Emits raw GTFN source.
`src/gt4py/next/program_processors/runners/roundtrip.py`	Revises source/module cache keys.
`src/gt4py/cartesian/backend/gtc_common.py`	Centralizes C++ formatting.
`src/gt4py/cartesian/backend/gtcpp_backend.py`	Removes scattered formatting.
`src/gt4py/cartesian/backend/dace_backend.py`	Removes scattered DaCe formatting.
`src/gt4py/cartesian/backend/numpy_backend.py`	Uses the new Python formatter.
`src/gt4py/cartesian/backend/debug_backend.py`	Uses the new Python formatter.
`src/gt4py/cartesian/backend/module_generator.py`	Preserves definitions verbatim.
`src/gt4py/cartesian/gtc/gtcpp/gtcpp_codegen.py`	Returns raw generated C++.
`tests/eve_tests/unit_tests/test_formatting.py`	Tests formatter success and fallback.
`tests/next_tests/integration_tests/cases_utils.py`	Updates formatter import.
`tests/next_tests/unit_tests/otf_tests/binding_tests/test_cpp_interface.py`	Updates C++ normalization helper.
`tests/next_tests/unit_tests/program_processor_tests/runners_tests/dace_tests/test_dace_bindings.py`	Updates Python normalization helper.
`docs/development/ADRs/next/0012-GridTools_Cpp_OTF_Steps.md`	Records removal of Next formatting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

egparedes · 2026-09-28T17:02:10Z

+    cache_key = (source_code, debug)
+    if cache_key in _MODULE_CACHE:
+        return _MODULE_CACHE[cache_key]


Added in b657b5d: tests/next_tests/unit_tests/program_processor_tests/runners_tests/test_roundtrip.py loads the same source with debug=False then True, and the reverse. It asserts that separate modules are created, that each is reused from the cache on a second load, and that only the debug module has a real .py __file__.

egparedes · 2026-09-28T16:59:13Z

cscs-ci run default

…odule cache - cartesian: 'format_source' controls C++ formatting in 'BaseGTBackend._make_extension_sources'. - cartesian: stencil-definition sources are kept verbatim. - next: roundtrip module cache is keyed by (source, debug); only debug modules are backed by a real '.py' file.

gt4py.cartesian keeps its original formatting call sites, options and line lengths; it only switches from `eve.codegen.format_source` to `eve.formatting.format_cpp_source` / `format_python_source`. Drop the cartesian tests covering the reverted behaviour. `format_python_source` gains a `line_length` argument and pins black's target version to the running interpreter, as the old formatter did.

egparedes · 2026-09-28T19:31:24Z

cscs-ci run default

Copilot

Copilot review overview

🟡 Changes recommended

The Python formatter can propagate unexpected Black failures, and two specified fallback/cache behaviors lack regression coverage.

Review effort: Balanced
Findings: 1 · 2

Open (3)

Catch unexpected formatter failures · New
Test unsupported interpreter fallback · New
Test module cache separation for debug mode

Resolved since last review (3)

Document removal of public codegen formatting API
Test verbatim preservation of stencil-definition sources
Cover format_source option for computation and binding sources

Previously missed (1)

In code that hasn't changed since last review

Test debug-independent source caching

src/gt4py/next/program_processors/runners/roundtrip.py:133

Removing debug from the source-cache key is not covered by the new test, which only exercises _MODULE_CACHE. Add a parametrized test that clears _SOURCE_CACHE, generates the same program in both debug orders, and asserts identical source plus a single cache entry; otherwise a future debug-dependent source/cache split can regress unnoticed.

egparedes · 2026-10-01T13:30:09Z

+    except ValueError:  # `black.InvalidInput` for unparsable source
+        return source


Fixed in af4457a: format_python_source now catches any Exception from black.format_str and returns the input unchanged, matching the old format_source(skip_errors=True) path. Added test_format_python_source_black_failure, which monkeypatches black.format_str to raise RuntimeError. It fails with the old except ValueError.

egparedes · 2026-10-01T13:30:10Z

+    except KeyError:  # `black` is too old to know the running interpreter
+        return source


Added in af4457a: test_format_python_source_unsupported_interpreter replaces black.TargetVersion with an empty mapping, so the running-version lookup raises KeyError, and checks the input comes back unchanged. The same commit also covers the "previously missed" source-cache point from the review overview. test_generate_source_ignores_debug_flag in test_roundtrip.py generates the same program with both debug orders and asserts identical source, a single transform call and a single _SOURCE_CACHE entry.

Restores the catch-all of the removed codegen.format_source(skip_errors=True) path used by gt4py.cartesian. Adds tests for unexpected black errors, an interpreter unknown to black, and the debug-independent roundtrip source cache.

…ormatting # Conflicts: # pyproject.toml # uv.lock

FlorianDeconinck

Good for cartesian, thanks!

edopao

Looks good, just some minor comments.

edopao · 2026-10-02T07:42:20Z

+    args = [
+        os.getenv("CLANG_FORMAT_EXECUTABLE", "clang-format"),
+        "--style=LLVM",
+        "--assume-filename=_gt4py_generated_file.cpp",
+    ]


I do not really understand why we expose CLANG_FORMAT_EXECUTABLE, but if we do it we should do the same check as in baseline, that the tool exists:

def _get_clang_format() -> Optional[str]: """Return the clang-format executable, or None if not available.""" executable = os.getenv("CLANG_FORMAT_EXECUTABLE", "clang-format") try: assert isinstance(executable, str) if subprocess.run([executable, "--version"], capture_output=True).returncode != 0: return None except Exception: return None return executable

The env var is kept for parity with the previous eve.codegen behaviour that cartesian relies on. Availability is now probed once with --version, like before (54b9caf). A missing or failing tool falls back to the unformatted source; see test_format_cpp_source_missing_executable, test_format_cpp_source_failing_executable, test_format_cpp_source_formatting_failure and test_format_cpp_source_probes_executable_once.

- Import the gt4py.eve.formatting module instead of its functions in next test helpers (cases_utils.debug_itir, test_cpp_interface).

Mirror the previous eve.codegen behaviour: check CLANG_FORMAT_EXECUTABLE once with --version (lazily, cached) and skip C++ formatting when the tool is unavailable. Addresses review comment on PR #2927.

edopao

LGTM

egparedes requested a balanced review from Copilot September 28, 2026 16:16

Copilot started reviewing on behalf of egparedes September 28, 2026 16:17 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

egparedes added 2 commits September 28, 2026 19:01

egparedes requested a balanced review from Copilot October 1, 2026 13:14

Copilot started reviewing on behalf of egparedes October 1, 2026 13:14 View session

egparedes mentioned this pull request Oct 1, 2026

feat[next]: add FORMAT_SOURCES config option to control formatting of generated code #2919

Closed

2 tasks done

Copilot AI reviewed Oct 1, 2026

View reviewed changes

egparedes marked this pull request as ready for review October 1, 2026 13:26

egparedes requested a review from edopao October 1, 2026 15:01

Merge remote-tracking branch 'upstream/main' into drop-next-codegen-f…

f76f828

…ormatting # Conflicts: # pyproject.toml # uv.lock

egparedes requested a review from romanc October 2, 2026 07:06

FlorianDeconinck reviewed Oct 2, 2026

View reviewed changes

edopao reviewed Oct 2, 2026

View reviewed changes

egparedes added 2 commits October 2, 2026 10:28

refactor: address review comments

62519c4

- Import the gt4py.eve.formatting module instead of its functions in next test helpers (cases_utils.debug_itir, test_cpp_interface).

refactor[eve]: probe clang-format availability once before formatting

54b9caf

Mirror the previous eve.codegen behaviour: check CLANG_FORMAT_EXECUTABLE once with --version (lazily, cached) and skip C++ formatting when the tool is unavailable. Addresses review comment on PR #2927.

edopao approved these changes Oct 2, 2026

View reviewed changes

egparedes merged commit 4e564e0 into main Oct 2, 2026
32 checks passed

egparedes mentioned this pull request Oct 2, 2026

cartesian: computation_src cache in _make_extension_sources never hits (backend_data returns a copy) #2932

Open

		except ValueError: # `black.InvalidInput` for unparsable source
		return source

		except KeyError: # `black` is too old to know the running interpreter
		return source

Conversation

egparedes commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Breaking changes / notes

Requirements

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

egparedes Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

egparedes commented Sep 28, 2026

Uh oh!

egparedes commented Sep 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

egparedes Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

egparedes Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

FlorianDeconinck left a comment

Choose a reason for hiding this comment

Uh oh!

edopao left a comment

Choose a reason for hiding this comment

Uh oh!

edopao Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

egparedes Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

edopao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

egparedes commented Sep 28, 2026 •

edited

Loading