Merge pull request #198 from khaeru/fix/write-xml-version

Write .model.Version to SDMX-ML

Author: khaeru

doc/api/model-common-list.rst

@@ -82,4 +82,5 @@
 :obj:`~.common.VTLConceptMapping`
 :obj:`~.common.VTLDataflowMapping`
 :obj:`~.common.VTLMappingScheme`
+:obj:`~.common.Version`
 :obj:`~.common.VersionableArtefact`

doc/api/writer.rst

@@ -38,11 +38,11 @@ Some of the internal methods take specific arguments and return varying values.
 These arguments can be passed to :func:`.to_pandas` when `obj` is of the appropriate type:
 
 .. autosummary::
-   sdmx.writer.pandas.write_dataset
-   sdmx.writer.pandas.write_datamessage
-   sdmx.writer.pandas.write_itemscheme
-   sdmx.writer.pandas.write_structuremessage
-   sdmx.writer.pandas.DEFAULT_RTYPE
+   write_dataset
+   write_datamessage
+   write_itemscheme
+   write_structuremessage
+   DEFAULT_RTYPE
 
 Other objects are converted as follows:
 

doc/howto/create.rst

@@ -63,14 +63,16 @@ There are different classes to describe dimensions, measures, and attributes.
 
     import sdmx
     from sdmx.model.v21 import (
+        Agency,
         DataStructureDefinition,
         Dimension,
         PrimaryMeasure,
         DataAttribute,
     )
 
     # Create an empty DSD
-    dsd = DataStructureDefinition(id="CUSTOM_DSD")
+    m = Agency(id="EXAMPLE")
+    dsd = DataStructureDefinition(id="CUSTOM_DSD", maintainer=m)
 
     # Add 1 Dimension object to the DSD for each dimension of the data.
     # Dimensions must have a explicit order for make_key(), below.

doc/whatsnew.rst

@@ -3,8 +3,10 @@
 What's new?
 ***********
 
-.. Next release
-.. ============
+Next release
+============
+
+- Bug fix for writing :class:`.VersionableArtefact` to SDMX-ML 2.1: :class:`KeyError` was raised if :attr:`.VersionableArtefact.version` was an instance of :class:`.Version` (:pull:`198`).
 
 v2.18.0 (2024-10-15)
 ====================

sdmx/message.py

@@ -285,12 +285,12 @@ def get(
             :attr:`~.IdentifiableArtefact.id`; if :class:`str`, an object with this ID
             *or* this string as part of its :attr:`~.IdentifiableArtefact.urn`.
 
+            .. todo:: Support passing a URN.
+
         Returns
         -------
         .IdentifiableArtefact
-            with the given ID and possibly class.
-        None
-            if there is no match.
+            with the given ID and possibly class, or :any:`None` if there is no match.
 
         Raises
         ------
@@ -299,8 +299,6 @@ def get(
             of different classes, or two objects of the same class with different
             :attr:`~.MaintainableArtefact.maintainer` or
             :attr:`~.VersionableArtefact.version`.
-
-        .. todo:: Support passing a URN.
         """
         id_ = (
             obj_or_id.id

sdmx/model/common.py

@@ -31,17 +31,20 @@
 
 from sdmx.dictlike import DictLikeDescriptor
 from sdmx.rest import Resource
+from sdmx.urn import URN
 from sdmx.util import compare, direct_fields, only
 
 from .internationalstring import (
     DEFAULT_LOCALE,
     InternationalString,
     InternationalStringDescriptor,
 )
+from .version import Version
 
 __all__ = [
     "DEFAULT_LOCALE",
     "InternationalString",
+    "Version",
     # In the order they appear in this file
     "ConstrainableArtefact",
     "Annotation",
@@ -238,25 +241,20 @@ class IdentifiableArtefact(AnnotableArtefact):
     #: a URN.
     urn: Optional[str] = None
 
-    urn_group: dict = field(default_factory=dict, repr=False)
-
     def __post_init__(self):
-        if not isinstance(self.id, str):
+        # Validate URN, if any
+        self._urn = URN(self.urn)
+
+        if not self.id:
+            self.id = self._urn.item_id or self._urn.id or MissingID
+        elif self.urn and self.id not in (self._urn.item_id or self._urn.id):
+            # Ensure explicit ID is consistent with URN
+            raise ValueError(f"ID {self.id} does not match URN {self.urn}")
+        elif not isinstance(self.id, str):
             raise TypeError(
                 f"IdentifiableArtefact.id must be str; got {type(self.id).__name__}"
             )
 
-        if self.urn:
-            import sdmx.urn
-
-            self.urn_group = sdmx.urn.match(self.urn)
-
-        try:
-            if self.id not in (self.urn_group["item_id"] or self.urn_group["id"]):
-                raise ValueError(f"ID {self.id} does not match URN {self.urn}")
-        except KeyError:
-            pass
-
     def __eq__(self, other):
         """Equality comparison.
 
@@ -362,23 +360,23 @@ def __repr__(self) -> str:
 @dataclass
 class VersionableArtefact(NameableArtefact):
     #: A version string following an agreed convention.
-    version: Optional[str] = None
+    version: Union[str, Version, None] = None
     #: Date from which the version is valid.
     valid_from: Optional[str] = None
     #: Date from which the version is superseded.
     valid_to: Optional[str] = None
 
     def __post_init__(self):
         super().__post_init__()
-        try:
-            if self.version and self.version != self.urn_group["version"]:
-                raise ValueError(
-                    f"Version {self.version} does not match URN {self.urn}"
-                )
-            else:
-                self.version = self.urn_group["version"]
-        except KeyError:
-            pass
+
+        if not self.version:
+            self.version = self._urn.version
+        elif isinstance(self.version, str) and self.version == "None":
+            self.version = None
+        elif self.urn and self.version != self._urn.version:
+            raise ValueError(
+                f"Version {self.version!r} does not match URN {self.urn!r}"
+            )
 
     def compare(self, other, strict=True):
         """Return :obj:`True` if `self` is the same as `other`.
@@ -420,15 +418,14 @@ class MaintainableArtefact(VersionableArtefact):
 
     def __post_init__(self):
         super().__post_init__()
-        try:
-            if self.maintainer and self.maintainer.id != self.urn_group["agency"]:
+
+        if self.urn:
+            if self.maintainer and self.maintainer.id != self._urn.agency:
                 raise ValueError(
                     f"Maintainer {self.maintainer} does not match URN {self.urn}"
                 )
             else:
-                self.maintainer = Agency(id=self.urn_group["agency"])
-        except KeyError:
-            pass
+                self.maintainer = Agency(id=self._urn.agency)
 
     def compare(self, other, strict=True):
         """Return :obj:`True` if `self` is the same as `other`.

sdmx/model/version.py

@@ -35,19 +35,26 @@ def cmp(self, other) -> bool:
 class Version(packaging.version.Version):
     """Class representing a version.
 
-    This class extends :class:`packaging.version.Version`, which provides a complete
-    interface for interacting with Python version specifiers. The extensions implement
-    the particular form of versioning laid out by the SDMX standards. Specifically:
-
-    - :attr:`kind` to identify whether the version is an SDMX 2.1, SDMX 3.0, or Python-
-      style version string.
+    The SDMX Information Model **does not** specify a Version class; instead,
+    :attr:`.VersionableArtefact.version` is described as “a version **string** following
+    SDMX versioning rules.”
+
+    In order to simplify application of those ‘rules’, and to handle the differences
+    between SDMX 2.1 and 3.0.0, this class extends :class:`packaging.version.Version`,
+    which provides a complete interface for interacting with Python version specifiers.
+    The extensions implement the particular form of versioning laid out by the SDMX
+    standards. Specifically:
+
+    - :attr:`kind` as added to identify whether a Version instance is an SDMX 2.1, SDMX
+      3.0, or Python-style version string.
     - Attribute aliases for particular terms used in the SDMX 3.0 standards:
       :attr:`patch` and :attr:`ext`.
     - The :class:`str` representation of a Version uses the SDMX 3.0 style of separating
       the :attr:`ext` with a hyphen ("1.0.0-dev1"), which differs from the Python style
       of using no separator for a ‘post-release’ ("1.0.0dev1") or a plus symbol for a
       ‘local part’ ("1.0.0+dev1").
-    - The class is comparable with :class:`str` version expressions.
+    - :meth:`increment`, an added convenience method.
+    - The class is comparable and interchangeable with :class:`str` version expressions.
 
     Parameters
     ----------

sdmx/tests/model/test_common.py

@@ -5,7 +5,7 @@
 import pytest
 
 import sdmx.model as model
-from sdmx.model import v21
+from sdmx.model import common, v21
 from sdmx.model.common import (
     Agency,
     AnnotableArtefact,
@@ -16,7 +16,6 @@
     IdentifiableArtefact,
     Item,
     ItemScheme,
-    MaintainableArtefact,
     NameableArtefact,
     Representation,
 )
@@ -90,18 +89,22 @@ def test_eval_annotation(self, caplog) -> None:
         )
 
 
+URN = "urn:sdmx:org.sdmx.infomodel.conceptscheme.ConceptScheme=IT1:VARIAB_ALL(9.6)"
+
+
 class TestIdentifiableArtefact:
-    def test_general(self):
-        urn = (
-            "urn:sdmx:org.sdmx.infomodel.conceptscheme.ConceptScheme=IT1:VARIAB_ALL"
-            "(9.6)"
-        )
-        urn_pat = urn.replace("(", r"\(").replace(")", r"\)")
+    def test_init_urn(self):
+        """IdentifiableArtefact can be initialized with URN."""
+        ia = IdentifiableArtefact(urn=URN)
+        assert "VARIAB_ALL" == ia.id
+
+    def test_general(self) -> None:
+        urn_pat = URN.replace("(", r"\(").replace(")", r"\)")
 
         with pytest.raises(
             ValueError, match=f"ID BAD_URN does not match URN {urn_pat}"
         ):
-            IdentifiableArtefact(id="BAD_URN", urn=urn)
+            IdentifiableArtefact(id="BAD_URN", urn=URN)
 
         # IdentifiableArtefact is hashable
         ia = IdentifiableArtefact()
@@ -162,27 +165,32 @@ def test_namea(self, caplog) -> None:
         assert na1.compare(na2)
 
 
-class TestMaintainableArtefact:
-    def test_urn(self):
-        urn = (
-            "urn:sdmx:org.sdmx.infomodel.conceptscheme.ConceptScheme="
-            "IT1:VARIAB_ALL(9.6)"
-        )
-        ma = MaintainableArtefact(id="VARIAB_ALL", urn=urn)
+class TestVersionableArtefact:
+    def test_urn(self) -> None:
+        va = common.VersionableArtefact(id="VARIAB_ALL", urn=URN)
 
         # Version is parsed from URN
-        assert ma.version == "9.6"
+        assert va.version == "9.6"
 
         # Mismatch raises an exception
-        with pytest.raises(ValueError, match="Version 9.7 does not match URN"):
-            MaintainableArtefact(version="9.7", urn=urn)
+        with pytest.raises(ValueError, match="Version '9.7' does not match URN"):
+            common.VersionableArtefact(version="9.7", urn=URN)
+
+    def test_version_none(self) -> None:
+        va = common.VersionableArtefact(version="None")
+        assert va.version is None
+
+
+class TestMaintainableArtefact:
+    def test_urn(self) -> None:
+        ma = common.MaintainableArtefact(id="VARIAB_ALL", urn=URN)
 
         # Maintainer is parsed from URN
         assert ma.maintainer == Agency(id="IT1")
 
         # Mismatch raises an exception
         with pytest.raises(ValueError, match="Maintainer FOO does not match URN"):
-            MaintainableArtefact(maintainer=Agency(id="FOO"), urn=urn)
+            common.MaintainableArtefact(maintainer=Agency(id="FOO"), urn=URN)
 
 
 class TestItem:

sdmx/tests/test_model.py

@@ -96,6 +96,8 @@
     # Appearing in model.InternationalString
     "DEFAULT_LOCALE",
     "InternationalString",
+    # Appearing in model.Version
+    "Version",
     # Classes that are distinct in .model.v21 versus .model.v30
     "SelectionValue",
     "MemberValue",

sdmx/tests/writer/test_writer_xml.py

@@ -7,6 +7,7 @@
 import sdmx
 import sdmx.writer.xml
 from sdmx import message
+from sdmx.model import common
 from sdmx.model import v21 as m
 from sdmx.model.v21 import DataSet, DataStructureDefinition, Dimension, Key, Observation
 from sdmx.writer.xml import writer as XMLWriter
@@ -166,14 +167,23 @@ def test_reference() -> None:
     assert 'version="1.0"' in result_str
 
 
-def test_Footer(footer):
-    """:class:`.Footer` can be written."""
-    sdmx.to_xml(footer)
+def test_VersionableArtefact() -> None:
+    """:class:`VersionableArtefact` with :class:`.Version` instance can be written."""
+    cl: common.Codelist = common.Codelist(id="FOO", version=common.Version("1.2.3"))
+
+    # Written to XML without error
+    result = sdmx.to_xml(cl).decode()
+    assert 'version="1.2.3"' in result
 
 
 # sdmx.message classes
 
 
+def test_Footer(footer):
+    """:class:`.Footer` can be written."""
+    sdmx.to_xml(footer)
+
+
 def test_structuremessage(tmp_path, structuremessage):
     result = sdmx.to_xml(structuremessage, pretty_print=True)