Skip to content

👌 ShellCode: drop redundant Model override#116

Merged
GeigerJ2 merged 2 commits into
aiidateam:masterfrom
GeigerJ2:fix/installedcode-model-rename
May 21, 2026
Merged

👌 ShellCode: drop redundant Model override#116
GeigerJ2 merged 2 commits into
aiidateam:masterfrom
GeigerJ2:fix/installedcode-model-rename

Conversation

@GeigerJ2
Copy link
Copy Markdown
Contributor

@GeigerJ2 GeigerJ2 commented May 20, 2026

commit msg:

aiida-core PR 6990 restructured the pydantic model layout on `Code`
classes: the single nested `Model` class is split into `CommonFields`,
`AttributesModel`, and `ConstructorArgsModel`. `ShellCode` overrides
these to set `default_calc_job_plugin = 'core.shell'` for attribute
loading and the `verdi code create` CLI default.

PR 6990 is not in any released `aiida-core` yet (as of 2026-05-20), so
keep both code paths: feature-detect the new API via the presence of
`aiida.orm.pydantic.OrmMetadataField` (added in 6990) and fall back to
the old single-class `Model` shape on releases that predate the
refactor. The dependency pin stays at `aiida-core~=2.6,>=2.6.1`.

@GeigerJ2 GeigerJ2 mentioned this pull request May 20, 2026
GeigerJ2 added a commit to GeigerJ2/aiida-core that referenced this pull request May 20, 2026
aiida-core aiidateam#6990 renamed `InstalledCode.Model` to `ReadModel` (and split
out `WriteModel`, `ConstructorModel`, `CliModel`), breaking the import
chain that module 3 executes on Read the Docs:

  aiida_workgraph -> aiida_shell.ShellJob -> ShellCode -> Model

aiidateam/aiida-shell#116 adds feature-detected dual-API support. Pin to
its fork branch (`GeigerJ2/aiida-shell@fix/installedcode-model-rename`)
until that PR is released so the tutorial notebooks under
`docs/source/tutorials/` execute on RTD again. Mirrors the existing
`aiida-workgraph` fork pin in the same `tutorials` extra.

Also fold in the ruff `I001` import-sort fix on
`docs/source/tutorials/include/tasks.py` that pre-commit auto-applied:
`aiida` is treated as first-party in aiida-core, so `from aiida import
...` must follow the third-party `from aiida_workgraph import ...`.
aiida-core PR 6990 restructured the pydantic model layout on `Code`
classes: the single nested `Model` class is split into `CommonFields`,
`AttributesModel`, and `ConstructorArgsModel`. `ShellCode` overrides
these to set `default_calc_job_plugin = 'core.shell'` for attribute
loading and the `verdi code create` CLI default.

PR 6990 is not in any released `aiida-core` yet (as of 2026-05-20), so
keep both code paths: feature-detect the new API via the presence of
`aiida.orm.pydantic.OrmMetadataField` (added in 6990) and fall back to
the old single-class `Model` shape on releases that predate the
refactor. The dependency pin stays at `aiida-core~=2.6,>=2.6.1`.
@GeigerJ2 GeigerJ2 force-pushed the fix/installedcode-model-rename branch from b6ead20 to 6a0876e Compare May 20, 2026 13:28
@GeigerJ2 GeigerJ2 requested a review from edan-bainglass May 20, 2026 13:29
Copy link
Copy Markdown
Member

@edan-bainglass edan-bainglass left a comment

Choose a reason for hiding this comment

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

Thanks @GeigerJ2, but actually, I think maybe we just remove the model entirely from aiida-shell? It adds nothing new, right? default_calc_job_plugin is already available in the root AbstractCode model.

aiida-core PR 6990 restructured the pydantic model layout on `Code`
classes: the single nested `Model` class is split into `CommonFields`,
`AttributesModel`, and `ConstructorArgsModel`. This broke
`ShellCode.Model(InstalledCode.Model)` on releases that include 6990,
since `InstalledCode.Model` no longer exists.

The override only ever set `default_calc_job_plugin = 'core.shell'`,
which is functionally redundant: `_to_orm_field_values` filters `None`
values out of the model -> ORM kwargs dict in both the old and new
aiida-core layouts, so when no value is passed at the CLI/Pydantic
boundary the `ShellCode.__init__` signature default of `'core.shell'`
takes over. Dropping the override removes the breakage on 6990 without
needing version-specific shims, and works unchanged on every aiida-core
that exposes a `_to_orm_field_values`-style filtering step (i.e., every
aiida-core that ever had a code Model).

The only observable difference is cosmetic: `verdi code create
core.code.shell --help` no longer shows `core.shell` as the printed
default for `--default-calc-job-plugin`. The constructed code still has
it set correctly.
GeigerJ2 added a commit to GeigerJ2/aiida-core that referenced this pull request May 20, 2026
aiidateam/aiida-shell#116 was updated to drop the `ShellCode.Model`
override entirely instead of feature-detecting both aiida-core model
APIs. The pin in `pyproject.toml` already points at
`GeigerJ2/aiida-shell@fix/installedcode-model-rename` (branch name
unchanged), so this commit only re-locks `uv.lock` to the new HEAD on
that branch.

The simpler approach works because both old and new aiida-core filter
`None` field values in their model -> ORM conversion, so the `__init__`
default of `'core.shell'` is the actual source of the behavior; the
nested `Model` override only ever shadowed that default for the `verdi
code create core.code.shell --help` cosmetic display.
@GeigerJ2 GeigerJ2 changed the title 👌 ShellCode: support both aiida-core model APIs 👌 ShellCode: drop redundant Model override May 20, 2026
@GeigerJ2
Copy link
Copy Markdown
Contributor Author

Thanks @GeigerJ2, but actually, I think maybe we just remove the model entirely from aiida-shell? It adds nothing new, right? default_calc_job_plugin is already available in the root AbstractCode model.

He, good catch! You're right, I pushed a follow-up that drops the Model override entirely instead of feature-detecting both APIs.

I walked through the construction paths in both old and new aiida-core to convince myself nothing breaks:

  • default_calc_job_plugin is declared on AbstractCode (old: Model, new: CommonFields) with default None.
  • cls.from_model(model)_from_cli_model(model) (new) or _from_model(model) (old) → fields = model._to_orm_field_values()cls(**fields).
  • Both old (model_to_orm_field_values, entities.py) and new (_to_orm_field_values, pydantic.py) filter out None values with the same if field_value is None: continue line. So default_calc_job_plugin=None never reaches the constructor.
  • ShellCode.__init__ still has default_calc_job_plugin: str = 'core.shell' in its signature, so when the field is filtered out, the right value is applied. validate_default_calc_job_plugin is still enforced for any explicitly-passed non-core.shell value.

I confirmed against an installed venv: verdi code create core.code.installed.shell --help shows -P, --default-calc-job-plugin TEXT with no printed default, which is the only observable change. The actual constructed code still has default_calc_job_plugin='core.shell' set correctly because the __init__ default takes over once None is filtered out.

Copy link
Copy Markdown
Member

@edan-bainglass edan-bainglass left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM!

@GeigerJ2 GeigerJ2 merged commit e2f4ff1 into aiidateam:master May 21, 2026
6 checks passed
@GeigerJ2 GeigerJ2 deleted the fix/installedcode-model-rename branch May 21, 2026 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants