Skip to content

refactor(config): simplify model and dataset target types to str - #173

Merged
aaishwarymishra merged 2 commits into
pytorch-ignite:mainfrom
webdevsamran:refactor/config-str-types
Sep 7, 2026
Merged

aaishwarymishra merged 2 commits into
pytorch-ignite:mainfrom
webdevsamran:refactor/config-str-types

Conversation

@webdevsamran

Copy link
Copy Markdown
Contributor

Follow up on discussion in #31.

Description

Currently, several Pydantic configuration classes in trainite/config/models.py and trainite/config/datasets.py type their target (_target_) fields as multi-line Literal[...] unions duplicating full import paths ("trainite.models..." vs "models..."). This makes the config definitions unnecessarily verbose and rejects custom user-provided implementations with strict validation errors.

Following maintainer guidance (@aaishwarymishra in #31), this PR:

  1. Replaces Literal[...] on target fields across trainite/config/models.py and trainite/config/datasets.py with str.
  2. Preserves default values pointing to canonical module paths.
  3. Adds unit test coverage in tests/config/base_test.py ensuring all model and dataset configs instantiate cleanly with string targets.

@aaishwarymishra
aaishwarymishra self-requested a review September 7, 2026 11:35
Comment thread tests/config/base_test.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I dont think we need test for this

@webdevsamran
webdevsamran force-pushed the refactor/config-str-types branch from c85f9a1 to a560862 Compare September 7, 2026 14:54
@webdevsamran

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @aaishwarymishra! Removed the test additions as requested - the PR now only touches trainite/config/datasets.py and trainite/config/models.py (2 files, 9 insertions / 32 deletions). Force-pushed to the same branch.

@aaishwarymishra
aaishwarymishra merged commit 2e4c2d3 into pytorch-ignite:main Sep 7, 2026
4 checks passed
@aaishwarymishra

Copy link
Copy Markdown
Member

Thanks :)

omarfathy7 pushed a commit to omarfathy7/trainite that referenced this pull request Sep 7, 2026
TahaZahid05 added a commit that referenced this pull request Sep 9, 2026
* docs: add configuration and training guide (#156)

* test: validate component registry (#157)

* test: validate component registry

* test: validate component registry

---------

Co-authored-by: Aaishwarya Mishra <[email protected]>

* refactor(config): simplify model and dataset target types to str (#173)

Co-authored-by: Aaishwarya Mishra <[email protected]>

* docs: fix component targets and config examples in training guide

* docs: trim _target_ section in training guide

* docs: restore dataset block in config example

* docs: simplify counting example description

---------

Co-authored-by: Taha Zahid <[email protected]>
Co-authored-by: Mohammed Adel <[email protected]>
Co-authored-by: Aaishwarya Mishra <[email protected]>
Co-authored-by: Samran Asif <[email protected]>
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