ITADN

validation handling of `visit_repeat_nodes` results in incorrect validation

#2000Openbraingram 创建于 2026-02-11
B
braingramcommented
I will work on a minimum example that can be turned into a unit test. The issue came up when https://github.com/astropy/asdf-astropy/pull/270 introduced the table-1.3.0 schema which uses tag validators: https://github.com/astropy/asdf-astropy/blob/8d798e24c2521e9646cb2d41e8c6487f96dbb611/asdf_astropy/resources/schemas/table/table-1.3.0.yaml#L103 for checking columns instead of `$ref`s to column-like schemas as is done in table-1.2.0: https://github.com/astropy/asdf-astropy/blob/8d798e24c2521e9646cb2d41e8c6487f96dbb611/asdf_astropy/resources/schemas/table/table-1.2.0.yaml#L103 When a table contains a column with an invalid unit (in this case a `magunit` which isn't supported which is a separate bug) the validator bug pops up. When a magunit is serialized it produced a non-unit-schema conforming representation (maybe also a separate bug). This results in a tagged tree for the table that should be invalid since it contains a `{'unit': '3.6307805e-20erg.Hz**-1.s**-1.cm**-2'}` where a string should be. However during validation asdf incorrectly marks this as valid because: - the column instance and column schema are validated as part of the table `anyOf` where the validation fails but asdf marks the instance/schema pair as "seen" - then when the column instance and column schema are validated due to the instance having the column tag validated is skipped because the instance/schema pair was "seen" above We can't just revisit all nodes (since that produces recursion errors) but if we remove that check and revisit nodes (for testing): https://github.com/asdf-format/asdf/actions/runs/21912149854/job/63268918611?pr=1999#step:10:2491 we see the error: ``` [79](https://github.com/asdf-format/asdf/actions/runs/21912149854/job/63268918611?pr=1999#step:10:2480) E Failed validating 'anyOf' in schema['properties']['columns']['items']: E {'anyOf': [{'$ref': 'http://stsci.edu/schemas/asdf/core/column-1.0.0'}, E {'$ref': 'http://stsci.edu/schemas/asdf/table/column-1.1.0'}, E {'$ref': 'http://stsci.edu/schemas/asdf/core/ndarray-1.1.0'}, E {'$ref': 'http://stsci.edu/schemas/asdf/time/time-1.2.0'}, E {'$ref': 'http://stsci.edu/schemas/asdf/unit/quantity-1.2.0'}, E {'$ref': '../coordinates/skycoord-1.0.0'}, E {'$ref': '../coordinates/earthlocation-1.1.0'}, E {'$ref': '../time/timedelta-1.1.0'}, E {'$ref': 'ndarraymixin-1.0.0'}]} E E On instance['columns'][30]: E {'data': {'byteorder': 'little', E 'datatype': 'float32', E 'shape': [0], E 'source': 30}, E 'description': 'AB magnitude within the elliptical Kron aperture', E 'name': 'forced_kron_abmag', E 'unit': {'unit': '3.6307805e-20erg.Hz**-1.s**-1.cm**-2'}} ``` where the validation is now correctly marking this as an invalid column serialization.
4 条评论