nexusformat / definitions

Definitions of the NeXus Standard File Structure and Contents
https://manual.nexusformat.org/
Other
26 stars 55 forks source link

NXsubentry documentation updates #1286

Open phyy-nx opened 1 year ago

phyy-nx commented 1 year ago

Closes #1124

phyy-nx commented 1 year ago

@prjemian can you review? I don't know if this helps address all the concerns in #1124.

prjemian commented 1 year ago

@gerrit-guenther: Can you comment?

prjemian commented 1 year ago

I'm not clear how the one line of documentation added by commit satisfies the questions raised in #1124 or https://github.com/nexusformat/definitions/issues/886#issuecomment-804191753.

phyy-nx commented 1 year ago

Ok, Ive re-read the original issue, and here are the specific questions, broken out and restated:

  1. Should NXsubentry only be used when the instrument would apply to different techniques/application definitions?
  2. Is NXsubentry intended to be a general approach (e.g. using a NXfluo subentry in a NXfluo entry)?
  3. What about the entry/definition attribute which is mandatory - what to write here if there are two subentries for e.g. SAXS/WAXS?
  4. Since the sample is defined for each subentry the subentry can apply to different measurements, different setups... Are there suggestions when to use NXentry or NXsubentry?

I've added new documentation to NXsubentry which I believe answers them.

  1. The primary goal of NXsubentry is to enable multi-model experiments. This is now stated directly: "Group to enable the combination of multiple application definitions for “multi-modal” measurements (e.g. multi-technique methods like SAXS/WAXS)."
  2. The two supported use cases are either an NXentry with an application definition, or an NXentry without an application definition and multiple sub entries with application definitions. This is now called out directly using a modified version of @prjemian's comment.

3 and 4 are hopefully answered by 2. In addition I've noted that NXsubentries aren't designed to be contained within each other.

Ok, these are at least my interpretations of what is going on. Feedback appreciated :)

phyy-nx commented 1 year ago

From code camp:

I have included this note in the PR: "An NXsubentry should not contain another NXsubentry". Is that correct? @yayahjb says we shouldn't need the restriction. @PeterC-DLS says it doesn't make sense for subentries to be nested. This would open the possibility of an application specifying a subentry. (Aaron: this might be useful for workflows?)

I based the assertion on my reading of the docs and @prjemian's comment showing the two use cases (with and without subentries) as meaning those are the only supported, but this might not be what was intended.

@mkoennecke what is the behavior of cnxvalidate if it finds a nested subentry?

Thoughts?