Validate properties attribute in output_schema #839
+111
−6
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in materialsproject/atomate2#1353 and materialsproject/atomate2#1354, currently a code like
Fails with an error:
but if the
(output_schema=M)is removed from the job definition the code is executed correctly, meaning that it is just the validation step that fails to identify the existence of the properties.This PR aims at making the code above correct even when the schema is enforced, by checking the existence of property-like attributes (e.g.
@property, functools'@cached_property, monty's@lazy_property) matching the name in the output schema.The current implementation does not attempt to extract the output type of the object returned by the property to verify if it is itself a subclass of
BaseModel. I tried playing a bit with extracting information from the typing of the method, but it seems hard to cover all the possible cases and is probably a less interesting case to cover. It should be easier to cover at least the standard@property, if this is seen as necessary.Another option would be to implement a
@typed_property(output_type=....)decorator, for the cases where it may be important to enforce the checks in the output model.