Handle type refs in _map_refs #166
Conversation
jacoblurye
force-pushed
the
handle-type-refs
branch
from
9c5e98f
to
ca536a2
Compare
There was a problem hiding this comment.
@jacoblurye No major comments, seems like an elegant solution.
One question is do we know have tests that would detect the reversion (at least for the template generation)?
|
@jim-bo we don't have tests for things like comments or validations appearing in Excel documents - I don't know of a way to automatically test that sort of thing, though. |
|
|
||
| # Plus concatenated new and old '$comment' fields | ||
| # which should be just ignored anyways. | ||
| if '$comment' in new_node or '$comment' in node: |
There was a problem hiding this comment.
As comments contain development related TODOs and stuff and we're now sending links to these docs to people, I think we should leave comments to ourselves and not add them in a visible manner to the docs.
| # Look for all refs in this mapping | ||
| for k, v in node.items(): | ||
| node[k] = _map_refs(v, on_refs) | ||
| if isinstance(node, collections.abc.Mapping): |
There was a problem hiding this comment.
I'm not sure how this works, I though map_refs will be called whenever json schema walker walks into a node with '$ref'. So from that understanding it will not be called at all on some node : {"type_ref": "..."}
There was a problem hiding this comment.
I think _map_refs is the json schema walker you're referring to? On the next line, we look for $refs and type_refs, then handle each case slightly differently. The test test_json_validation.test_map_refs might be a bit helpful, too.
The _map_ref's update does two main things:
type_reftype_ref(now validations and comments should appear)Also, makes a little tweak to
macros.j2to handleconsts in schemas.