Throw warning for variadic notation for non-variadic names

[lukaspie]

No description provided.

[rettigl]

I generally support this, but maybe we only merge this once we have sibling inheritance properly solved?

[rettigl]

"/ENTRY/CALIBRATION[transmission_correction]"
This should not give a warning, no?

WARNING: Given group name 'CALIBRATION' conflicts with the non-variadic name 'transmission_correction (opt)', which should be of type NXcalibration.

[lukaspie]

I generally support this, but maybe we only merge this once we have sibling inheritance properly solved?

Yeah, that was the idea 👍 just wanted to have a starting point for this.

"/ENTRY/CALIBRATION[transmission_correction]"
This should not give a warning, no?
WARNING: Given group name 'CALIBRATION' conflicts with the non-variadic name 'transmission_correction (opt)', which should be of type NXcalibration.

That is indeed weird. Will need to check what is going on here.

[lukaspie]

"/ENTRY/CALIBRATION[transmission_correction]" This should not give a warning, no?

WARNING: Given group name 'CALIBRATION' conflicts with the non-variadic name 'transmission_correction (opt)', which should be of type NXcalibration.

This should be allowed now, but it goes a bit against the logic we enforce elsewhere. Since transmission_correction is a non-variadic concept, there should ideally not be a concept name given. I have implemented now that you can give the concept names for a non-variadic group if the concept name matches the group's type (i.e., CALIBRATION matches for NXcalibration).

However, if your template has

"/ENTRY[my_entry]/CALIBRATION[transmission_correction]/<some-field>" (<some-field> being required)
"/ENTRY[my_entry]/transmission_correction/<another-field>"

we would still get a warning like "The data entry corresponding to ENTRY[my_entry]/transmission_correction/<some-field> is required". This is because the get_variations_of function for transmission_correction stops at the second key and doesn't consider the one with the concept name given. We could change get_variations_of, but still each variant gets checked separately. So you either need to use CALIBRATION[transmission_correction] OR transmission_correction for all subelements.

See also this test and the one above.

So, the question is, do we even allow this? May be confusing if you write the template like above and you get a warning even though each element in the template is technically correct.

The same will happen if you have (assuming sibiling inheritance works)

"/ENTRY[my_entry]/data/AXISNAME[energy]"
"/ENTRY[my_entry]/data/energy/@type"

This will tell you that the @type attribute is written for a non-existing energy field.

[rettigl]

What should this do for fields with given concept name? I was expecting similar behavior, but off course the type cannot be checked.
For keys
"/ENTRY/identifierNAME[identifier_entry]": "@attrs:metadata/loader/scan_path",
"AXISNAME_indices[@_indices]": "@DaTa:.index",
I still get the warnings:
WARNING: Given field name 'identifierNAME' conflicts with the non-variadic name 'identifier_entry (opt)'
WARNING: Field /ENTRY[entry]/identifierNAME[identifier_entry] written without documentation.
WARNING: Given attribute name 'AXISNAME_indices' conflicts with the non-variadic name '@energy_indices (rec)'
WARNING: Attribute /ENTRY[entry]/data/AXISNAME_indices[@energy_indices] written without documentation.

In particular the last one is not so easy to solve if we want to keep the "*" notation, as we cannot define all potential axisname_indices, yet want to define a few one, so they should be addressable using the same notation to be able to use the "*" construct.

[rettigl]

So, the question is, do we even allow this? May be confusing if you write the template like above and you get a warning even though each element in the template is technically correct.

Off course it would be helpful if there was not this restriction in resolution, but I think if this is difficult to implement, we could keep this and just add a small note in the documentation about the conversion template formalism.

[lukaspie]

What should this do for fields with given concept name? I was expecting similar behavior, but off course the type cannot be checked. For keys "/ENTRY/identifierNAME[identifier_entry]": "@attrs:metadata/loader/scan_path", "AXISNAME_indices[@__indices]": "@DaTa:_.index", I still get the warnings: WARNING: Given field name 'identifierNAME' conflicts with the non-variadic name 'identifier_entry (opt)' WARNING: Field /ENTRY[entry]/identifierNAME[identifier_entry] written without documentation. WARNING: Given attribute name 'AXISNAME_indices' conflicts with the non-variadic name '@energy_indices (rec)' WARNING: Attribute /ENTRY[entry]/data/AXISNAME_indices[@energy_indices] written without documentation.

In particular the last one is not so easy to solve if we want to keep the "" notation, as we cannot define all potential axisname_indices, yet want to define a few one, so they should be addressable using the same notation to be able to use the "" construct.

In the current version, NXentry/identifier_entry is indeed a non-variadic term, so this is still just the problem with sibling inheritance, right? I was planning to address this next. Same for @energy_indices and @AXISNAME_indices.

[lukaspie]

Closing here as all changes were brought to #621