nexpy / nxvalidate

NeXus validation tools and libraries
Other
0 stars 3 forks source link

NXcollection is an invalid class in NXsensor #6

Closed jkotan closed 1 month ago

jkotan commented 2 months ago

I get the following warning NXcollection is an invalid class in NXsensor

I think NXcollection is allowed below NXsensor

rayosborn commented 2 months ago

I have just released v0.2.0b1. Please check to see if this is still a bug.

jkotan commented 2 months ago

The bug is still in v0.2.0b1

rayosborn commented 2 months ago

I just checked the NXsensor NXDL file, and there is no mention of NXcollection and the ignoreExtraGroups attribute has not been set. Technically, this means that the presence of NXcollection groups in NXsensor groups is a violation. Whether this should be a violation is something to raise with the NIAC. One of the aims of NXvalidate is to trigger such discussions. It might be argued that NXcollection groups should be allowed everywhere, but I'm not sure if the current NXDL syntax has a mechanism for defining such rules.

If you think the above statement is incorrect, please explain why.

jkotan commented 2 months ago

According to

https://github.com/nexusformat/definitions/blob/9059815e131dd308677d01da793b63f9d214dbb3/manual/source/design.rst?plain=1#L479-L520

NXcollection, NXdata , NXlog , ... can appear anywhere in the NeXus hierarchy, where needed.

rayosborn commented 1 month ago

This should now be fixed in v0.3.0-beta.1, at least when using the definitions stored in the package. The NIAC recently approved the addition of inheritance to the NXobject class, which now means that it is possible to add fields and classes that are allowed in every base class to the NXobject NXDL file. In NXValidate, these classes include NXcollection, NXdata, NXlog, NXnote, and NXparameters. I should probably add NXtransformations, but it wasn't specified in the list I was able to compile.

This should also fix #8 and #9.

jkotan commented 1 month ago

Thanks. Indeed, in the newest version the issues #6, #8, #9 are fixed (if one use internal definitions)