Separate Generated Data And Dimension Namespaces

[MuellerSeb]

Closes #65

This merge request separates schema properties, runtime dimensions, and
generated operations into distinct namespaces in generated Fortran types.

Generated namelist objects now use public companion types:

type, public :: nml_run_data_t
  ! schema properties
end type nml_run_data_t

type, public :: nml_run_dims_t
  ! runtime dimensions
end type nml_run_dims_t

type, public :: nml_run_t
  type(nml_run_data_t) :: data
  type(nml_run_dims_t) :: dims
  logical :: is_configured = .false.
contains
  ! existing type-bound procedures
end type nml_run_t

Schemas without runtime dimensions omit the dimensions companion type and
dims component.

This is an intentional beta API break for native Fortran consumers. Direct
field access changes from config%field to config%data%field, and runtime
dimension access changes from config%n_items to config%dims%n_items. No
compatibility aliases or temporary generation mode are provided.

Changes

• Added stable public <namelist>_data_t and optional <namelist>_dims_t
  companion types.
• Moved all schema-backed values below %data and configured runtime
  dimensions below %dims, while retaining lifecycle state and type-bound
  procedures on the outer object.
• Added targeted, case-insensitive collision checks for companion type names
  against generated and imported module symbols.
• Removed the former restrictions caused by the flat outer member namespace:
  • properties and dimensions may use generated binding names such as set,
    from_file, is_set, and is_valid;
  • properties and dimensions may use outer member names such as data,
    dims, and is_configured; and
  • a property and runtime dimension may have the same name.
• Reserved errmsg case-insensitively for schema properties and runtime
  dimensions, with a focused diagnostic, so every public procedure can retain
  the existing errmsg= keyword.
• Centralized generated storage expressions so initialization, defaults,
  allocation, setters, derived values, presence checks, shape queries,
  constraint validation, and f2py wrappers consistently address %data or
  %dims.
• Isolated generated procedure state with nml__* names, including the
  passed-object dummy, function results, I/O state, and generated locals.
• Kept direct intrinsic calls and documented a focused, case-insensitive
  reservation list for identifiers such as present, size, and shape
  that would shadow generated dependencies.
• Made the generated helper source an explicit required output for native
  Fortran and f2py generation; application modules remain supported only for
  imported derived types and configured kinds.
• Split from_file(...) into its existing public type-bound wrapper and a
  private module reader helper. The helper owns schema-spelled namelist locals,
  preserving flat native namelist syntax for fields named file, status,
  nml, iostat, or close_status.
• Added deterministic mangling for collision-prone internal f2py ABI names,
  including schema fields and dimensions named status, handle, or after
  Fortran intrinsics. Python-facing parameter names and behavior remain
  unchanged.
• Made generated array storage explicitly one-based by contract:
  • fixed declarations and allocations use default lower bounds;
  • index validation compares against 1:size(...) extents;
  • flexible-array scans and populated-prefix validation use one-based
    sections; and
  • partial setters assign into 1:size(value, dim) destinations.
• Removed generated lower/upper-bound temporaries and owned-storage
  lbound(...)/ubound(...) bookkeeping.
• Renamed generated runtime-dimension default symbols to
  <dimension>__dim_default, preventing a same-named schema property default
  from colliding at module scope.
• Updated the README, manual Fortran consumers, optional-reader fixture, golden
  outputs, and all committed generated examples for the new component paths.

Behavior Notes

• Native namelist input remains flat. Fields are still written as field = ...
  or derived%component = ..., never as data%field = ....
• Schema paths, template names, Markdown names, is_set(...) and
  filled_shape(...) lookup strings, and Python-facing APIs are unchanged.
• The native set, set_dims, from_file, is_set, filled_shape, and
  is_valid interfaces retain their existing behavior and public error model.
  Status results remain integers; this change does not introduce a status
  derived type.
• Failed set_dims(...) calls remain transactional. Candidate dimensions are
  validated before either %dims or dependent %data storage is changed.
• Assumed-shape setter inputs may have arbitrary caller-side lower bounds;
  shape-based assignment still produces one-based generated destination
  storage.
• Generated allocatable components are public, but manually reallocating them
  with non-one lower bounds is unsupported. Generated initialization and
  dimension changes restore the supported one-based layout.

Migration

Native Fortran consumers must update direct component access:

! Before
iterations = config%iterations
n_items = config%n_items

! After
iterations = config%data%iterations
n_items = config%dims%n_items

Callers with many reads can use associate to keep access concise:

associate(data => config%data)
  iterations = data%iterations
  tolerance = data%tolerance
end associate

Calls to the existing type-bound procedures and Python wrappers do not require
a corresponding migration.

Tests

Added focused generator coverage for:

• companion declarations with and without runtime dimensions;
• nested storage expressions across initialization, allocation, setters,
  derived values, presence, shape, and validation paths;
• generated outer-member, procedure-local, property/dimension, companion-type,
  and imported-symbol collisions;
• targeted case-insensitive rejection of errmsg;
• direct-intrinsic reservation diagnostics and private reader generation;
• one-based index, scan, prefix, and partial-assignment expressions without
  lower/upper-bound bookkeeping; and
• collision-safe f2py ABI names with unchanged Python-facing names.

Added a compiled Fortran namespace conformance fixture covering:

• properties and dimensions named after generated members and procedure state;
• data, dims, property/dimension overlap, status, file, nml,
  iostat, and close_status, with direct-intrinsic names rejected separately;
• set, set_dims, flat from_file, presence and validity queries;
• successful and failed transactional dimension updates; and
• one-based generated storage populated from caller arrays with non-one lower
  bounds.

[Copilot]

🟡 Changes recommended

Unresolved generator and wrapper compilation issues, plus validation and dimension-order defects, remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Separates generated Fortran schema data and runtime dimensions into public companion namespaces while preserving flat namelist syntax and Python-facing APIs.

Changes:

• Adds %data and optional %dims companion types with one-based storage.
• Hardens symbol handling, reader state, intrinsic aliases, and f2py ABI names.
• Updates generators, tests, fixtures, examples, documentation, and CI.

File summaries

File	Description
tests/test_schema.py	Tests schema identifier validation.
tests/test_enum_support.py	Updates enum generation coverage.
tests/test_codegen_fortran.py	Tests namespaces, collisions, and storage behavior.
tests/test_codegen_f2py.py	Tests f2py ABI name handling.
tests/test_cli_config.py	Tests dimension default naming.
tests/fortran_optional_read/out/nml_required.f90	Updates generated required reader output.
tests/fortran_optional_read/out/nml_optional.f90	Updates generated optional reader output.
tests/fortran_optional_read/out/nml_helper.f90	Updates generated helper output.
tests/fortran_optional_read/conformance.f90	Migrates optional-reader consumers.
tests/fortran_namespaces/out/nml_namespaces.f90	Adds generated namespace layout.
tests/fortran_namespaces/out/nml_helper.f90	Updates namespace helper output.
tests/fortran_namespaces/nml-config.toml	Configures namespace generation.
tests/fortran_namespaces/namespaces.yml	Defines namespace test schema.
tests/fortran_namespaces/input.nml	Provides namespace test input.
tests/fortran_namespaces/conformance.f90	Exercises namespace conformance.
tests/fortran_namespaces/CMakeLists.txt	Builds namespace conformance tests.
tests/fixtures/01_simple/out/nml_optimization.md	Updates generated documentation fixture.
tests/fixtures/01_simple/out/nml_optimization.f90	Updates generated module fixture.
tests/fixtures/01_simple/out/nml_helper.f90	Updates generated helper fixture.
src/nml_tools/templates/python_wrappers.py.j2	Preserves Python-facing wrapper names.
src/nml_tools/templates/nml_helper.f90.j2	Exports intrinsic helper aliases.
src/nml_tools/schema.py	Validates reserved identifiers.
src/nml_tools/codegen_f2py.py	Mang​les internal f2py identifiers.
src/nml_tools/cli.py	Renames dimension default symbols.
src/nml_tools/_utils.py	Supports identifier and dimension validation.
README.md	Documents namespace migration and storage behavior.
examples/04_derived_types/fortran/generated/nml_run.f90	Updates generated derived-type module.
examples/04_derived_types/fortran/generated/nml_helper.f90	Updates generated helper output.
examples/04_derived_types/fortran/config_store.f90	Migrates native field access.
examples/03_references/out/nml_run.f90	Updates generated run module.
examples/03_references/out/nml_report.f90	Updates generated report module.
examples/03_references/out/nml_helper.f90	Updates generated helper output.
examples/02_pybind/fortran/generated/nml_helper.f90	Updates generated helper output.
examples/02_pybind/fortran/generated/nml_config.f90	Updates generated configuration module.
examples/02_pybind/fortran/config_store.f90	Migrates native field access.
examples/01_simple/out/nml_helper.f90	Updates generated helper output.
examples/01_simple/main.f90	Migrates native field access.
.github/workflows/ci.yml	Updates CI configuration.

Review details

Suppressed comments (2)

src/nml_tools/codegen_fortran.py:473

• runtime_dimensions is appended only when a shape is encountered, so its order follows schema/property traversal rather than the configured [dimensions] mapping. If the config declares n_b before n_a but the schema first uses n_a, the public *_dims_t layout and positional set_dims argument order no longer match the documented configuration order; register or reorder used dimensions from runtime_dimension_values before emitting these APIs.

            index += 1

    def _register_runtime_dimension(dim_name: str) -> str:
        local_name = runtime_dimension_locals.get(dim_name)
        if local_name is None:
            default_name = _generated_name(dim_name, "dim_default")
            if default_name.lower() in static_constants:

src/nml_tools/schema.py:285

• This validation is only reached when _resolve_node runs; resolve_mapping returns the original schema early when it has no references or derived objects requiring normalization. As a result, a plain root schema with a property named errmsg still resolves successfully, despite the new policy reserving that name for every schema property (the added test uses a nested object and therefore misses this fast path). Validate properties in the pre-normalization identifier walk, or otherwise apply this check before the early return.

            validate_namelist_identifier(name, label=f"property '{name}'")

• Files reviewed: 24/41 changed files
• Comments generated: 3
• Review effort level: Lite

---

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

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate generator, collision-handling, and CLI configuration issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

src/nml_tools/cli.py:992

• This new guard allows configurations with no mod_path, but the later local_derived_types check still unconditionally raises when helper_path is absent. As a result, a docs/template-only schema containing a local x-fortran-type still fails even though no Fortran source needs the helper. The helper requirement needs to be gated consistently on native Fortran output, not just this initial check.

    if helper_path is None and any(
        loaded.entry["mod_path"] is not None for loaded in loaded_namelists
    ):
        raise click.ClickException("Fortran module generation requires [helper].path")

src/nml_tools/cli.py:1367

• The same output-mode mismatch remains in gen-fortran: this new check only requires a helper when a module path is present, while the later local-derived-type validation still rejects a no-mod_path entry. A configuration that emits no native Fortran module should not require a helper merely because its schema has a local derived type.

    f2cmap_path, f2py_c_types = _load_f2py_settings(config, base_dir)
    resolver = SchemaResolver()
    entries = _iter_namelists(config, base_dir)
    if helper_path is None and any(entry["mod_path"] is not None for entry in entries):
        raise click.ClickException("Fortran module generation requires [helper].path")

src/nml_tools/codegen_fortran.py:629

• This applies validate_namelist_identifier to every derived component, so a schema such as value%size or value%present is rejected even though generated references to those components are qualified (...%size) and cannot shadow the intrinsic. The documented reservation is for identifiers emitted unqualified; validate nested components with the general identifier/errmsg rules instead, otherwise valid derived-type schemas are unnecessarily unusable.

                    validate_namelist_identifier(
                        child_display_name,
                        label=f"property '{child_display_name}'",
                    )

src/nml_tools/schema.py:501

• The recursive schema validation applies the root-property reservation set to nested derived-type components. Consequently value%size and value%present cannot be resolved even though generated accesses are qualified and the README scopes these intrinsic/helper reservations to names emitted unqualified. Keep the nested-component check to valid identifiers plus the intentional errmsg/__ rules, or valid derived schemas will fail before code generation.

                        validate_namelist_identifier(name, label=f"property '{name}'")

• Files reviewed: 29/50 changed files
• Comments generated: 5
• Review effort level: Lite

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

🟡 Changes recommended

Two critical generated-Fortran name-collision issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 29/50 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved critical helper-symbol and moderate dimension-ordering findings, plus the broad generator/API change, require human review.

Review details

Suppressed comments (1)

src/nml_tools/codegen_fortran.py:461

• runtime_dimensions is appended when a shape is first encountered, so the public *_dims_t component order and the positional set_dims dummy order follow schema traversal rather than the [dimensions] configuration order promised by the new layout. If the config declares {b, a} but the schema first uses a, this emits a, b and changes positional structure-constructor/setter semantics; order the used dimensions by runtime_dimension_values before rendering.

    def _register_runtime_dimension(dim_name: str) -> str:
        local_name = runtime_dimension_locals.get(dim_name)
        if local_name is None:
            default_name = _generated_name(dim_name, "dim_default")
            if default_name.lower() in static_constants:

• Files reviewed: 29/50 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Two critical compile-time namespace collision issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 29/50 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/nml_tools/codegen_f2py.py:405

• c_intptr_t is used as the kind of every generated wrapper handle, but it is missing from this reserved-name set. A valid schema field or runtime dimension named c_intptr_t is therefore emitted as a wrapper dummy with the same name; the integer(c_intptr_t) :: nml__handle declaration then resolves the kind name to that dummy and the generated wrapper does not compile. Reserve it so it is ABI-mangled like status and handle.

        "this",

src/nml_tools/codegen_fortran.py:1547

• Runtime dimensions are not checked against the generated outer type name. A dimension named nml_run_t becomes a set_dims dummy while that function also declares class(nml_run_t), so the dummy shadows the type name and the generated module fails to compile. Include the outer type name in the runtime-dimension procedure-scope collision checks.

    if runtime_dimension_values:
        set_dims_name = f"{module_name}_set_dims"
        if set_dims_name.lower() in runtime_dimension_values:
            raise ValueError(
                f"runtime dimension '{set_dims_name}' conflicts with "
                f"generated set_dims procedure '{set_dims_name}'"

• Files reviewed: 29/50 changed files
• Comments generated: 5
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The unresolved critical f2py naming collision can prevent generated wrappers from compiling, with an additional internal naming nit outstanding.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/nml_tools/templates/fortran_module.f90.j2:157

• This resolver still exposes handle and ptr as unprefixed generated support names, even though this change otherwise reserves nml__* for internal procedure state. Rename these to collision-safe names such as nml__handle and nml__ptr (and update the two uses below) so the generated f2py helper follows the stated naming policy consistently and cannot reintroduce unqualified support names.

  subroutine {{ type_prefix }}_resolve_handle(handle, nml__obj, nml__status, errmsg)
    integer(c_intptr_t), intent(in) :: handle !< opaque handle to a {{ type_name }} instance
    type({{ type_name }}), pointer :: nml__obj !< resolved namelist pointer
    integer, intent(out) :: nml__status !< nml-tools status code
    character(len=*), intent(out), optional :: errmsg !< error message for non-OK status values
    type(c_ptr) :: ptr

• Files reviewed: 29/50 changed files
• Comments generated: 1
• Review effort level: Lite