search

FAIRmat-NFDI / data-modeling

3 stars 1 forks source link

Check IVTemp example to make sure it correctly complains on missing inherited User group #128

Open sanbrock opened 1 year ago

sanbrock commented 1 year ago

This is very similar to #112 and should be solved with it or afterwards.

sherjeelshabih commented 1 year ago

The converter does not have NXsensor_scan required fields in the generate-template. So it doesn't check for User group, if its not provided. But if a User group is provided, for an instance of NXiv_temp, it gets correctly identified and sorted.

The calibration_time field is not found for example: "/ENTRY[entry]/INSTRUMENT[instrument]/ENVIRONMENT[environment]/voltage_controller/calibration_time"

sherjeelshabih commented 1 year ago

It's possible to check whether an AppDef extends and incorporate the parent AppDef as well. The only question is how to handle mods. This is currently not clear how we could handle it, implementation-wise.

sanbrock commented 1 year ago

get_inherited_nodes on "/ENTRY/INSTRUMENT/ENVIRONMENT" works fine and finds inheritance from sensor_scan, too.

Problem is with the next step when looking for "/ENTRY/INSTRUMENT/ENVIRONMENT/voltage_controller" as it is not found in sensor_scan (because only 'SENSOR' is defined there whet we wanted to specialise in iv_temp by requiring the presence of 'voltage_controller').

To allow a new concept (intentionally having an IS_A or SAME_AS relationship to a superclass) be defined under a specialised name (as above), the inheritance check must include checking its type (base class name /NX_CLASS/ in case of a Group or data type in case of a Field) because it is needed to resolve ambiguity according to the NeXus naming convention. (e.g. two groups defined next to one another, but without specifying their names must come from different base classes)

Note that this information is present in the application definitions, so it should not be a problem to pass it over.

sanbrock commented 1 year ago

Actually, I have implemented it with no need to pass over the type, as the code now finds it automatically.

sherjeelshabih commented 1 year ago

Thank you very much! This should help with the merge and get all children function.

I also want to point out that there is one more inheritance issue with NXpid under NXenvironment. All children of the following cannot be found:

For example:

sherjeelshabih commented 1 year ago

The commit here actually generates the new template. But this now fails the previously setup pytests. If you like I can update the tests. But first if someone could have another look at the generated templates, it will be nice to see what we have.

The template generation fails for further groups found in parent classes that haven't been mentioned in the AppDef. This happens when trying to access NXgeometry somehow. I didn't have a deeper look. It's better to check if we do need more in this template generation.

sanbrock commented 1 year ago

"/ENTRY[entry]/INSTRUMENT[instrument]/ENVIRONMENT[environment]/NXpid[heating_pid]"
should read
"/ENTRY[entry]/INSTRUMENT[instrument]/ENVIRONMENT[environment]/PID[heating_pid]"

sherjeelshabih commented 1 year ago

This is found correctly. Thanks.

sherjeelshabih commented 1 year ago

The relevant MR is here.