oumi
oumi copied to clipboard
oumi-ai
feat: Updated BaseConfig class for non primitive fields
Description
This is a PR for the issue 1628. The to_yaml does not warn when trying to save non primitive data types (such as functions). This leads to errors while loading from saved yaml files.
I have added a check to gracefully check for non primitive types recursively. If present, they are removed from the config. The removed types and their paths are specified to the user via logs.
Related issues
Fixes # (issue) https://github.com/oumi-ai/oumi/issues/1628
Before submitting
- [ ] This PR only changes documentation. (You can ignore the following checks in that case)
- [x] Did you read the contributor guideline Pull Request guidelines?
- [x] Did you link the issue(s) related to this PR in the section above?
- [x] Did you add / update tests where needed?
Reviewers
At least one review from a member of oumi-ai/oumi-staff is required.
Thank you @abhiramvad for the contribution! Given this changes a very sensitive class, would you be able to add unit tests to validate the logic ?
I made the changes requested previously in comments. I have added few baseline tests, checking the test_config.py . Hope they align with what is needed. Kindly provide me comments on those. I'll update them soon.
Hi @abhiramvad , could you please fix the precommit errors? You can verify it passes locally by running pre-commit run --all-files --show-diff-on-failure.
Thank you @abhiramvad for the updates! I think this is very close to the finish line. It looks like some of the new tests are failing?
@abhiramvad could you fix the test error? You can run the checks locally with pre-commit run --all-files --show-diff-on-failure. Please re-request review when all tests pass locally.
@wizeng23 i fixed these errors. Here is a precommit check screenshot from my machine
The tests work on my machine, and i also ran the CPU tests workflow on my fork. Lets hope it doesn't break here... https://github.com/abhiramvad/oumi/actions/runs/18120697923