Skip to content

Conversation

@ganatraad
Copy link
Contributor

@ganatraad ganatraad commented Dec 14, 2025

What does this PR do?

Fixes #823

To summarize, if the config class has a dict member, then the override was of the form
--cfg.dict '{"key": val}'
This PR allows passing in --cfg.dict.key val
The existing syntax is still valid.

Before submitting

  • Did you read the contributing guideline?
  • Did you update the documentation? (readme and public docstrings)
  • Did you write unit tests such that there is 100% coverage on related code? (required for bug fixes and new features)
  • Did you verify that new and existing tests pass locally?
  • Did you make sure that all changes preserve backward compatibility?
  • Did you update the CHANGELOG including a pull request link? (not for typos, docs, test updates, or minor internal changes/refactors)

Copy link
Member

@mauvilsa mauvilsa left a comment

Choose a reason for hiding this comment

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

Thank you so much for contributing! The fix in _typehints.py file looks good.

Please:

  • Change the pull request title to something that summarizes what is being fixed.
  • In the changelog only a single entry with a description similar to the pull request title and a link only to the pull request, not the issue. Also it should be in the already existing Fixed subsection.
  • Add a unit test, that should fail without the _typehints.py fix.

@ganatraad ganatraad changed the title Issue-823 Dict override syntax to accept standard form: --dict.key val Dec 23, 2025
@ganatraad ganatraad changed the title Dict override syntax to accept standard form: --dict.key val Dict override syntax to accept standard form: --cfg.dict.key val Dec 23, 2025
@ganatraad ganatraad changed the title Dict override syntax to accept standard form: --cfg.dict.key val Dict override syntax to allow standard form: --cfg.dict.key val Dec 23, 2025
@ganatraad
Copy link
Contributor Author

Addressed the comments cc @mauvilsa

Copy link
Member

@mauvilsa mauvilsa left a comment

Choose a reason for hiding this comment

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

The test is not the same case as #823. Please fix.

@codecov
Copy link

codecov bot commented Dec 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (10cfd30) to head (27edf8e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #824   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines         7234      7237    +3     
=========================================
+ Hits          7234      7237    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mauvilsa mauvilsa merged commit b7efc86 into omni-us:main Dec 25, 2025
29 checks passed
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.

Command line override for cfg.dict.dataclass.attribute

2 participants