Skip to content
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

TST: Add test to ensure all optional fields have defaults #834

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

tnatt
Copy link
Collaborator

@tnatt tnatt commented Oct 7, 2024

PR that adds a test (and relevant functionality) to check that all optional pydantic fields have default values specified.
Adresses #819

@tnatt tnatt self-assigned this Oct 7, 2024
@tnatt tnatt requested review from mferrera and slangeveld October 7, 2024 13:28
@tnatt tnatt force-pushed the test_optional_fields branch from dbd8c79 to cf3b1fe Compare October 7, 2024 13:30
@tnatt
Copy link
Collaborator Author

tnatt commented Oct 7, 2024

The tests should fail now since we have one Optional field without a default present in the schema.
However, did not expect the docs to fail.. will investigate . But maybe you have an idea what is going on here before I deep dive into it @mferrera ?

@tnatt
Copy link
Collaborator Author

tnatt commented Oct 7, 2024

Ok the docs failing is not related to this PR. Good 😄

Copy link
Collaborator

@mferrera mferrera left a comment

Choose a reason for hiding this comment

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

These tests will aid our inadequate human brains so much in the long run from making silly and forgetful but simulation killing mistakes. I imagine there are more we can build in as things come up -- i.e., models that derived from Data but are not placed into AnyData

for field_name, field_info in model.model_fields.items():
if (
type(None) in get_args(field_info.annotation)
and field_info.is_required()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did not know about is_required(), neat

@tnatt tnatt force-pushed the test_optional_fields branch 4 times, most recently from 9721e23 to faf488e Compare October 8, 2024 10:35
@tnatt tnatt force-pushed the test_optional_fields branch from faf488e to fa4e5be Compare October 10, 2024 07:50
@tnatt tnatt merged commit 1cc79a8 into equinor:main Oct 10, 2024
13 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.

2 participants