Return None for missing Optional field attribute access - #30
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A core use case for implicitdict has always been the ability to accurately represent JSON and be easily written to JSON. In native JSON objects (and therefore in derivatives like JSON Schema and OpenAPI), the difference between absence ("this field is not present") and a positive value of null ("this field contains a value of null") is important. Therefore, implicitdict was originally developed to strongly differentiate between these two states. Specifically, if an object had an Optional
foofield, thenobj.foowould raise an AttributeError whenobjdid not have itsfookey/field populated. This has caused a great deal of headache in practical usage since most (but not all) use cases, positively specifying null and omitting the field have the same effect. The headache manifests in two ways: first, it is easy to forget to check whether"foo" in objbefore attempting to accessobj.foo, so uss_qualifier has had a number of bugs where this checking step was omitted. Second,"foo" in objdoes not link"foo"with the actualfoofield definition, so searching for usages of thefoofield in most IDEs will not find"foo" in objand this is occasionally problematic.I realized that these headaches are not necessary evils to retain the full ability to handle explicitly-null and unspecified fields, and I believe this realization will resolve all headaches without loss of functionality. By having missing Optional fields return None from their attribute accessor, we can skip the
"foo" in objcheck and merely checkobj.foo is Nonein most cases. But, the ability to differentiate explicit-null from missing is still retained with the"foo" in objcheck (though I expect that check to be rare in most current usage). So, I think the change in this PR is a global win, though it does change behavior enough that I think it justifies a major revision in the semantic version upon next release.