Fix Measurement.to_xml() writing invalid LuminousIntensity on FilterM… - #26
Merged
vanous merged 2 commits intoJul 24, 2026
Conversation
…easurement Measurement.to_xml() wrote LuminousIntensity unconditionally whenever self.luminous_intensity was not None (default 0.0 on read). This is correct for EmitterMeasurement, which requires the attribute, but the same Measurement class also serializes FilterMeasurement, where gdtf.xsd does not declare LuminousIntensity as a permitted attribute at all - so a round-tripped Filter/Measurement became XSD-invalid even though the source file was valid. Transmission already guards against this one line below via _attr_keys (the set of attribute names actually present on the source element): only re-emit it if it was in the source, or its value is non-zero. This applies the identical condition to LuminousIntensity, so an EmitterMeasurement (which always has LuminousIntensity in valid GDTF) keeps writing it unchanged, while a FilterMeasurement (which never does) stops inventing one. Reproduction: round-trip any GDTF file with PhysicalDescriptions/ Filters/Filter/Measurement elements (e.g. the Robe Robin Esprite / iSpiider / MegaPointe files on gdtf-share.com) and validate against gdtf.xsd - each Filter/Measurement gains an invalid LuminousIntensity="0.000000".
Collaborator
|
Code needs to be formatted, thank you. |
Unrelated to the LuminousIntensity fix: the "Check if code is formatted" workflow runs ruff format --check with ruff "latest" (not the 0.11.5 pinned in .pre-commit-config.yaml). A newer ruff now reformats markdown-embedded Python code fences too, and CHANGELOG.md had drifted out of compliance on master itself (missing space after # and a stray blank line) - this was failing CI on every push/PR, including this one, independent of the actual change.
Contributor
Author
Thanks — turned out to be CHANGELOG.md, not the actual fix. The "Check if code is formatted" workflow runs ruff format --check with ruff latest, while .pre-commit-config.yaml pins ruff-pre-commit at v0.11.5. A newer ruff now also reformats Python code fences inside markdown, and CHANGELOG.md had drifted out of compliance on master itself (missing space after #, one stray blank line) — so the check was already red before this PR, independent of the LuminousIntensity/FilterMeasurement change. Pushed a small follow-up commit (2a455ae) fixing just that file; pygdtf/init.py and the new test were already compliant under both the pinned and latest ruff versions. All 19 tests still pass. |
vanous
pushed a commit
that referenced
this pull request
Jul 24, 2026
WheelSlot._read_xml() read a missing MediaFileName as "" instead of
None (xml_node.attrib.get("MediaFileName", "") defaults to an empty
string, not None). to_xml() only checks "is self.media_file_name not
None" - since a Resource("") was always constructed, every wheel slot
got MediaFileName="" written back out on round-trip, even slots that
never had that attribute in the source file.
Harmless to the XSD (empty string is still a valid attribute value),
but it pollutes every wheel slot on round-trip. Same root cause as the
LuminousIntensity fix in #26: read with a truthy default instead of
None, write unconditionally instead of checking whether the source
actually had the attribute.
Also includes the same CHANGELOG.md formatting fix as #26 (ruff
`latest` reformats markdown-embedded Python code fences; master is
currently failing "Check if code is formatted" independent of either
PR) so this PR's CI is green on its own regardless of merge order.
Contributor
Author
|
@vanous it was a pleasure to help, have a nice weekend! |
Collaborator
|
Thank you! |
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
2 participants
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.
…easurement
Measurement.to_xml() wrote LuminousIntensity unconditionally whenever self.luminous_intensity was not None (default 0.0 on read). This is correct for EmitterMeasurement, which requires the attribute, but the same Measurement class also serializes FilterMeasurement, where gdtf.xsd does not declare LuminousIntensity as a permitted attribute at all - so a round-tripped Filter/Measurement became XSD-invalid even though the source file was valid.
Transmission already guards against this one line below via _attr_keys (the set of attribute names actually present on the source element): only re-emit it if it was in the source, or its value is non-zero. This applies the identical condition to LuminousIntensity, so an EmitterMeasurement (which always has LuminousIntensity in valid GDTF) keeps writing it unchanged, while a FilterMeasurement (which never does) stops inventing one.
Reproduction: round-trip any GDTF file with PhysicalDescriptions/ Filters/Filter/Measurement elements (e.g. the Robe Robin Esprite / iSpiider / MegaPointe files on gdtf-share.com) and validate against gdtf.xsd - each Filter/Measurement gains an invalid LuminousIntensity="0.000000".