Make struct codec encode default values #1116
Conversation
benjeffery
force-pushed
the
struct-default
branch
from
c8274a9 to
67abae4
Compare
Codecov Report
@@ Coverage Diff @@
## main #1116 +/- ##
=======================================
Coverage 93.70% 93.70%
=======================================
Files 26 26
Lines 21050 21065 +15
Branches 894 899 +5
=======================================
+ Hits 19724 19739 +15
Misses 1289 1289
Partials 37 37
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
Looks good. I think we should validate the schema at construction time, though, so raise an error if MetadataSchema(schema) is called with conflicting required/default values.
python/tests/test_metadata.py
Outdated
| exceptions.MetadataSchemaValidationError, | ||
| match="Optional property 'int' must have a default value", | ||
| ): | ||
| self.encode(schema, {"float": 5.5}) |
There was a problem hiding this comment.
This should be raised when we create the schema rather than when encoding data I think? (probably is, but the test should reflect this)
There was a problem hiding this comment.
Modified test to just create schema, the schema was where the error was already being rasied.
| prop | ||
| for prop, sub_schema in ret.get("properties", {}).items() | ||
| if "default" not in sub_schema | ||
| ] |
There was a problem hiding this comment.
Can check here if any properties that don't have a default are in required too.
There was a problem hiding this comment.
That's checked on line 174
benjeffery
force-pushed
the
struct-default
branch
from
67abae4 to
572b137
Compare
benjeffery
force-pushed
the
struct-default
branch
from
572b137 to
8ccb607
Compare
|
@jeromekelleher Should be good to go now |
2 participants
Partial work for #1073 on the struct side.