feat: tighter validation on depends_on targets#71
Conversation
jokasimr
force-pushed
the
transformations-are-nxnumber
branch
from
a090a74 to
b594984
Compare
| ) | ||
|
|
||
| def validate(self, node: Dataset | Group) -> Violation | None: | ||
| if not node.attrs.get("NX_class") == "NXlog": |
There was a problem hiding this comment.
Shouldn't this do nothing if node is a Dataset?
Also, do you think it is worth validating the dtype of the Dataset (same above in is_complete_transformation).
There was a problem hiding this comment.
It's not very clear maybe but the validator does not apply to datasets.
Also, do you think it is worth validating the dtype of the Dataset (same above in is_complete_transformation).
I don't think so. I can add it if you want but it seems like a unlikely error.
There was a problem hiding this comment.
It's not very clear maybe but the validator does not apply to datasets.
Are you sure? It says in def applies_to(self, node: Dataset | Group) -> bool: that it applies to everything in NXtransformations parent groups?
There was a problem hiding this comment.
The condition in applies_to was isinstance(node, Group) before.
I've now changed it so that it applies to both groups and datasets.
| ) | ||
| bad = chexus.Group( | ||
| name="transformations/bad", attrs={"NX_class": "NXcollection"}, parent=parent | ||
| ) |
There was a problem hiding this comment.
Missing good and bad datasets?
There was a problem hiding this comment.
I've added a good dataset test, but there is no bad dataset case for this validator because we don't check dtype.
2 participants
This PR implements tighter validation on
depends_ontargets, and verifies children of transformation groups areNX_NUMBER.Supersedes #70