Skip to content

feat: eliminate requirement for custom root schema#108

Merged
HennerM merged 1 commit intomainfrom
feat/less-boilerplate
Mar 23, 2026
Merged

feat: eliminate requirement for custom root schema#108
HennerM merged 1 commit intomainfrom
feat/less-boilerplate

Conversation

@HennerM
Copy link
Copy Markdown
Collaborator

@HennerM HennerM commented Mar 6, 2026

Currently each project needs to specify a root config schema and override the schemas of components that have a custom configuration, for example when having datasets that require a custom config. Additionally for callbacks and normalizers the way to add a Callback is cumbersome because we

  1. create the callback class
  2. create the callback config class
  3. override the custom Trainer schema to allow the type of the new callback
  4. use the callback in the config yaml

The 3rd step is just extra plumbing that can be avoided.

This PR adds functionality to avoid having to create a custom root schema. It works by designating certain component configs as "Registries". Allowing all subclasses of these registries to be valid types for validation, chosen by the already used "kind" discriminator. The semantics are the same to before, we just save a few things to write.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

Coverage

Tests Skipped Failures Errors Time
1182 21 💤 0 ❌ 0 🔥 26.443s ⏱️

@HennerM HennerM force-pushed the feat/less-boilerplate branch 2 times, most recently from c840cc2 to 2f98d4f Compare March 6, 2026 13:27
Comment thread src/noether/core/schemas/lib.py Outdated
Comment thread src/noether/core/schemas/lib.py Outdated
Comment thread src/noether/core/schemas/lib.py Outdated
Comment thread src/noether/core/schemas/lib.py Outdated
@HennerM HennerM force-pushed the feat/less-boilerplate branch 3 times, most recently from b8bf647 to 17d133a Compare March 16, 2026 15:04
@HennerM HennerM changed the title feat: reduce schema boilerplate feat: eliminate requirement for custom root schema Mar 23, 2026
Copy link
Copy Markdown
Collaborator

@MauritsBleeker MauritsBleeker left a comment

Choose a reason for hiding this comment

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

I would also remove the root config for the tutorial to be consistent (and change the text accordingly if needed)

self,
callback_config: PeriodicDataIteratorCallbackConfig,
trainer: BaseTrainer,
trainer,
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.

Why do we remove the type hinting here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We can't import BaseTrainer normally because that creates a cyclic import. That's why we did a if TYPE_CHECKING import on the BaseTrainer. However, with that it's not actually imported at runtime. This leads to an error when reading the type hints from the init init_params = get_type_hints(cls_.__init__), which is done to find the schema for a given class.

I think the if TYPE_CHECKING is kind-of bad anyway. We could fix the cyclic import by moving the imports in the trainer/base.py file inside the methods. I did that before and that ended up in a handful of local imports. I figured it's easier to just remove the import in this file instead, since we don't really need the type-hint for trainer here anyway

@HennerM HennerM force-pushed the feat/less-boilerplate branch from 17d133a to 60d44f5 Compare March 23, 2026 08:49
@HennerM HennerM force-pushed the feat/less-boilerplate branch from 60d44f5 to ddd0885 Compare March 23, 2026 08:49
@HennerM HennerM merged commit 81a62ae into main Mar 23, 2026
9 checks passed
@HennerM HennerM deleted the feat/less-boilerplate branch March 23, 2026 10:21
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants