Incorrect depends_on logic in FormatNexus

[rjgildea]

FormatNexus appears to use incorrect depends_on logic to determine whether or not it is a still:

https://github.com/dials/dxtbx/blob/2caec59df9e63219892e152d47dbdef2afc845f4/format/FormatNexus.py#L172-L175

@depends_on is listed as "required" in the NXmx specification. As I read the specifications, it would be valid for still images to have @depends_on present but its value set to ".":

https://manual.nexusformat.org/classes/applications/NXmx.html

depends_on: (required) NX_CHAR

This is a requirement to describe for any scan experiment. The axis on which the sample position depends may be stored anywhere, but is normally stored in the NXtransformations group within the NXsample group.

If there is no goniometer, e.g. with a jet, depends_on should be set to “.”

[phyy-nx]

I agree with your reading. In fact, the SwissFEL JF16M does set the NXsample depend_on to ".": https://github.com/cctbx/cctbx_project/blob/master/xfel/swissfel/jf16m_cxigeom2nexus.py#L179

If I run dxtbx.print_header on the SwissFEL JF16M standard dataset, it uses FormatNexus and not FormatNexusStill. However it correctly reports no scan or gonio, so in practice it would be treated as a still image dataset.

With the following diff, FormatNexusStill is used instead:

$ git diff
diff --git a/format/FormatNexus.py b/format/FormatNexus.py
index db9fcb6..dde1559 100644
--- a/format/FormatNexus.py
+++ b/format/FormatNexus.py
@@ -17,6 +17,7 @@ from dxtbx.format.nexus import (
     NXmxReader,
     detectorgroupdatafactory,
     generate_scan_model,
+    h5str,
     is_nexus_file,
 )

@@ -171,7 +172,7 @@ class FormatNexusStill(FormatMultiImageLazy, FormatNexus, FormatStill):

                 for entry in find_entries(handle, "/"):
                     for sample in find_class(entry, "NXsample"):
-                        if "depends_on" not in sample:
+                        if "depends_on" not in sample or h5str(sample['depends_on'][()]) == ".":
                             is_nexus_still = True
         except IOError:
             return False

Is that along the lines of what you were thinking?

[rjgildea]

Yes, that change is along the lines of what I was thinking.

[rjgildea]

I note https://github.com/cctbx/dxtbx/pull/253 touches on the same bit of code:

https://github.com/cctbx/dxtbx/pull/253/files#diff-8afa78cc4e23e11f363ac468ba2e7e6481a7fc5f99fa2dc1fb19dfbb54bd7dd5R174-R177