From 4a74324ae98734f08b43af6d3f9697bab0361a0e Mon Sep 17 00:00:00 2001 From: Marvin Date: Thu, 23 Jul 2026 22:21:49 +0200 Subject: [PATCH] Fix WheelSlot writing invalid empty MediaFileName="" on every slot 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. --- CHANGELOG.md | 3 +- pygdtf/__init__.py | 5 +- tests/test_wheel_slot_media_file_name.py | 108 +++++++++++++++++++++++ 3 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 tests/test_wheel_slot_media_file_name.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 79e366d..a03f71e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,7 +164,6 @@ having to use the .utils package. See more details below. ```python modes_info = gdtf_fixture.dmx_modes.as_dict() - ``` ##### .utils.get\_geometry\_by\_name @@ -176,7 +175,7 @@ gdtf_fixture.geometries.get_geometry_by_name("geometry name") ##### .utils.get\_geometry\_by\_type ```python -#this is a static method and requires a root_geometry +# this is a static method and requires a root_geometry gdtf_fixture.geometries.get_geometry_by_type(geometry_root, geometry_type) ``` diff --git a/pygdtf/__init__.py b/pygdtf/__init__.py index 5509f82..867bedb 100644 --- a/pygdtf/__init__.py +++ b/pygdtf/__init__.py @@ -791,8 +791,9 @@ def _read_xml(self, xml_node: "Element", xml_parent: Optional["Element"] = None) color_str = xml_node.attrib.get("Color") self.color = ColorCIE(str_repr=color_str) if color_str else ColorCIE() self.filter = NodeLink("FilterCollect", xml_node.attrib.get("Filter")) - self.media_file_name = Resource( - name=xml_node.attrib.get("MediaFileName", ""), extension="png" + media_file_name = xml_node.attrib.get("MediaFileName") + self.media_file_name = ( + Resource(name=media_file_name, extension="png") if media_file_name else None ) self.facets = [PrismFacet(xml_node=i) for i in xml_node.findall("Facet")] animation_system_node = xml_node.find("AnimationSystem") diff --git a/tests/test_wheel_slot_media_file_name.py b/tests/test_wheel_slot_media_file_name.py new file mode 100644 index 0000000..fb2582c --- /dev/null +++ b/tests/test_wheel_slot_media_file_name.py @@ -0,0 +1,108 @@ +# MIT License +# +# Copyright (C) 2026 Marvin +# +# This file is part of pygdtf. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +from pathlib import Path +from xml.etree import ElementTree + +import pygdtf + +# WheelSlot._read_xml() read a missing MediaFileName as "" instead of None +# (default on xml_node.attrib.get), so to_xml() - which only checks "is this +# not None" - always wrote MediaFileName="" back out, even for slots that +# never had that attribute in the source. Harmless to the XSD (empty string +# is still a valid value) but pollutes every wheel slot on round-trip. +_DSC_XML = """ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +""" + + +def test_wheel_slot_roundtrip_omits_empty_media_file_name(tmp_path: Path): + dsc_file = tmp_path / "description.xml" + dsc_file.write_text(_DSC_XML, encoding="utf-8") + + with pygdtf.FixtureType(dsc_file=str(dsc_file)) as fixture: + writer = pygdtf.FixtureTypeWriter(fixture) + output = tmp_path / "roundtrip.xml" + writer.write_gdtf(output) + + root = ElementTree.fromstring(output.read_bytes()) + slots = root.findall("./FixtureType/Wheels/Wheel/Slot") + assert len(slots) == 2 + + open_slot, gobo_slot = slots + assert open_slot.get("Name") == "Open" + assert "MediaFileName" not in open_slot.attrib + + assert gobo_slot.get("Name") == "Gobo 1" + assert gobo_slot.get("MediaFileName") == "gobo01"