-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor tests pa #336
Refactor tests pa #336
Conversation
panosatha
commented
Jan 19, 2024
•
edited
Loading
edited
- Small updates in the benefits code to avoid repetative code and add documentation
- Refactoring of tests for benefits by implementing unit tests for each method
…into refactor_tests_PA
…into refactor_tests_PA
------- | ||
IBenefit | ||
a Benefit object | ||
""" | ||
|
||
obj = Benefit() | ||
with open(filepath, mode="rb") as fp: | ||
toml = tomli.load(fp) | ||
obj.attrs = BenefitModel.parse_obj(toml) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parse_object throws an error/warning The method "parse_obj" in class "BaseModel" is deprecated
The parse_obj
method is deprecated; use model_validate
instead
Do we want to update that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same in line 607 with "dict"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is the case for all our objects currently after a change in the pydantic version. So this should be solved in a seperate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a small test for "def save(self, filepath: Union[str, os.PathLike]): ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just had minor comments/questions