Detect cycles in depends_on #254
Conversation
|
|
||
| root = make_group(h5root) | ||
| with pytest.raises(ValueError, match="Circular depends_on"): | ||
| _ = root[()] |
There was a problem hiding this comment.
Thanks.
Did you try loading a NMX file from coda to check that the error is raised?
There was a problem hiding this comment.
I did now and it does find a cycle:
'/entry/instrument/detector_panel_0/transformations/orientation',
'/entry/instrument/detector_panel_0/transformations/axis6',
'/entry/instrument/detector_panel_0/transformations/axis5',
'/entry/instrument/detector_panel_0/transformations/axis4',
'/entry/instrument/detector_panel_0/transformations/axis3',
'/entry/instrument/detector_panel_0/transformations/axis2',
'/entry/instrument/detector_panel_0/transformations/axis1'
'/entry/instrument/detector_panel_0/transformations/stageZ'
'/entry/instrument/detector_panel_0/transformations/orientation'
Can you explain why you think this is a good approach? I thought it would be easy to misjudge a typo for a chain-end, i.e., say But maybe I misunderstood what you meant? |
|
This is what I meant. I don't have a strong opinion on this. But I figured that since we treat |
I guess close to 100% of the code or config files used to write files is written by hand. I am not sure what you are saying. I think self-loops are just as bad as other loops? |
We have integration tests with broken files that have circular
depends_ongraphs. ScippNeXus currently runs in an infinite loop in those cases.This PR adds
depends_onchains are detected and lead toValueErrors.depends_onare treated asdepends_on == '.'. While this does not comply with the standard, I see no reason not to do this.