Make Mono.d_spacing conditionally required per XDI/1.0 spec - #6
Merged
Merged
Conversation
The XDI/1.0 specification requires Mono.d_spacing only when the
abscissa (Column.1) is monochromator angle or encoder step count,
since d_spacing is needed to convert those units to energy:
"The Mono.d_spacing header field must be specified if the abscissa
is conveyed as monochromator angle."
-- specification/spec.md (XraySpectroscopy/XAS-Data-Interchange)
The schema previously required Mono.d_spacing unconditionally, causing
spurious errors on the many XDI files whose abscissa is energy or
wavelength (where d_spacing is optional).
Fix:
* Remove the unconditional `required: ["d_spacing"]` from the
`mono` subschema.
* Add a top-level `allOf` conditional that requires
`mono.d_spacing` only when `column.1` matches
`^(angle|encoder)(\s|$)`.
Tests:
* Existing `test_missing_mono` (fixture has energy abscissa) is
replaced with `test_mono_dspacing_not_required_for_energy_abscissa`
to reflect the corrected semantics.
* `test_WriteToFile` / `test_WriteToFileTwo`: their `invalid_data`
setup now also swaps the abscissa to `angle deg` so removing
d_spacing still triggers the mono error path.
* New `test_MonoDspacingConditional.py` with three cases:
- angle abscissa, no d_spacing -> mono error
- angle abscissa, with d_spacing -> no mono error
- energy abscissa, no d_spacing -> no mono error
* New fixtures `missing_dspacing_angle.xdi` and
`valid_angle_dspacing.xdi`.
Owner
|
Dear @smrgeoinfo, thank you very much for this important contribution. Just reviewed your PR and everything seems fine for me. I will merge it now and prepare a new release to make the changes promptly available for the other users. |
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.
Summary
The XDI/1.0 specification requires
Mono.d_spacingonly when the abscissa (Column.1) is monochromator angle or encoder step count — the mono d-spacing is needed only to convert those units into energy:The current schema requires
Mono.d_spacingunconditionally, which produces spurious errors on the many XDI files whose abscissa is energy or wavelength (whered_spacingis optional).Fix
"required": ["d_spacing"]from themonosubschema.allOfconditional that requiresmono.d_spacingonly whencolumn.1matches^(angle|encoder)(\s|$).Tests
test_missing_mono(whose fixture hasColumn.1: energy) is replaced withtest_mono_dspacing_not_required_for_energy_abscissato reflect the corrected semantics.test_WriteToFile/test_WriteToFileTwo:invalid_datasetup now also switches the abscissa toangle degso removingd_spacingstill triggers the mono error path.tests/test_MonoDspacingConditional.pycovers three cases:monoerrormonoerrormonoerrortests/missing_dspacing_angle.xdi,tests/valid_angle_dspacing.xdi.Test plan
python -m pytest tests/— 26/27 pass on this branch. The one remaining failure (test_WrongFields::test_invalid_column) is pre-existing onmain(unrelated to this change).mono.d_spacing: after the fix, no mono error is raised; when the same file's abscissa is edited toangle deg, the mono error correctly fires.Context
Encountered while integrating
xdi_validatorinto a CDIF-XAS metadata pipeline against a corpus of ~100 real-world XDI files from Diamond Light Source beamline B18 (all energy abscissa). Every file was being flagged as invalid solely on the unconditionalMono.d_spacingrequirement, which the XDI/1.0 spec does not actually mandate for energy abscissae.