Conversation
…sformat#1659) Remove two categories of symbol declarations identified in issue nexusformat#1659: Rec 1 — Dead symbols (zero uses in any <dim> element): - NXsample_component: remove entire <symbols> block (n_Temp, n_eField, n_mField, n_pField, n_sField never referenced) - NXdetector_channel: remove dataRank and nP; strip rank="dataRank" from the two <dimensions> blocks that referenced it - NXem_ebsd: remove n_op and n_z - NXem_eds: remove n_elements Rec 2 — Singleton symbols (each used in exactly one <dim>): Symbols that appear only once cannot enforce cross-field coordination, the sole purpose of a <symbols> block. They are removed and their meaning is described in prose instead. - NXsample: remove n_Temp, n_eField, n_mField, n_pField, n_sField; drop the <dimensions> block from each of the five affected fields; n_comp (17+ uses) is retained - NXcylindrical_geometry: remove entire <symbols> block (i, j, k); retain fixed second-dimension constraints where present - NXoff_geometry: remove i, k, l; also remove the undeclared j from winding_order; retain fixed second-dimension constraints - NXbeam: remove c; update incident_beam_divergence doc; the second dimension is left unconstrained (dim index="2" with no value) - NXelectronanalyzer: remove nfa and nsa; drop <dimensions> from fast_axes and slow_axes - NXcrystal: remove n_comp; retain the fixed <dim index="2" value="6"/> constraint on unit_cell - NXreflections: remove m; update experiments field doc - NXapm_instrument: remove p; update signal_amplitude field doc - NXapm_reconstruction: remove n; retain the fixed <dim index="2" value="3"/> constraint on reconstructed_positions - NXapm_charge_state_analysis: remove n_ivec_max and n_variable; second dimension of nuclide_hash is left unconstrained - NXdisk_chopper: remove n; document the 2×n_slits length in prose Rec 3 (mode-specific symbols to be moved to application definitions, e.g. nP/i/j/k/tof in NXdetector, nP/m in NXbeam, i in NXcrystal, nwl in NXguide) is left for a follow-up PR after NIAC review. All 1207 existing tests pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
In addition, this PR allows reusing symbols in derived and child classes: #1650. |
Contributor
|
The warning in nexusformat comes from the fixed rank in base classes that are not respected in the HDF5 files. Giving symbols to dimensions in base class fields does not hurt and might be used in application definitions to reference certain dimensions of fields in the base classes (used in fields defined in the application definition itself) without having to override those fields for the sole purpose of defining symbols. Of course when there are symbol collisions you will have the override those symbols in the application definition. |
This was referenced Oct 1, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1659.
Implements recommendations 1 and 2 from issue #1659, which identified inconsistencies in the use of
<symbols>blocks across NeXus base classes. The purpose of a symbols block is to assert that multiple fields share identically-sized dimensions; symbols that are dead (never referenced in any<dim>element) or singletons (used in exactly one field) do not serve that purpose and are removed.Recommendation 1 — Dead symbols (zero uses in any
<dim>element):NXsample_component: remove entire<symbols>block (n_Temp,n_eField,n_mField,n_pField,n_sFieldwere never referenced)NXdetector_channel: removedataRankandnP; striprank="dataRank"from the two<dimensions>blocks that used itNXem_ebsd: removen_opandn_zNXem_eds: removen_elementsRecommendation 2 — Singleton symbols (each used in exactly one
<dim>):NXsample: removen_Temp,n_eField,n_mField,n_pField,n_sField; replace<dimensions>blocks with prose;n_comp(17+ uses) is retainedNXcylindrical_geometry: remove entire<symbols>block (i,j,k); retain fixed second-dimension constraintsNXoff_geometry: removei,k,l; also remove the undeclaredjused inwinding_order; retain fixed second-dimension constraintsNXbeam: removec; second dimension ofincident_beam_divergenceleft unconstrained with updated doc;nPandm(multi-use) retainedNXelectronanalyzer: removenfaandnsa; drop<dimensions>fromfast_axesandslow_axesNXcrystal: removen_comp; retain the fixed<dim index="2" value="6"/>constraint onunit_cell;i(4 uses) retainedNXreflections: removem; updateexperimentsfield docNXapm_instrument: removep; updatesignal_amplitudefield docNXapm_reconstruction: removen; retain the fixed<dim index="2" value="3"/>constraint onreconstructed_positionsNXapm_charge_state_analysis: removen_ivec_maxandn_variable; second dimension ofnuclide_hashleft unconstrainedNXdisk_chopper: removen; document the 2×n_slits length ofslit_edgesin proseRecommendation 3 (moving mode-specific symbols such as
nP/i/j/k/tofinNXdetectorandNXbeamto the relevant application definitions) is deferred to a follow-up PR pending NIAC review.