From 466658d8a5f8ecc52fac79b8fefa4b7cb1ad949e Mon Sep 17 00:00:00 2001 From: NicolasGensollen Date: Tue, 22 Aug 2023 10:35:34 +0200 Subject: [PATCH 1/2] improve _check_derived_image_xml --- .../converters/adni_to_bids/adni_json.py | 63 ++++++++++++------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/clinica/iotools/converters/adni_to_bids/adni_json.py b/clinica/iotools/converters/adni_to_bids/adni_json.py index 21cb322fd..59dde45bc 100644 --- a/clinica/iotools/converters/adni_to_bids/adni_json.py +++ b/clinica/iotools/converters/adni_to_bids/adni_json.py @@ -102,31 +102,46 @@ def _get_text(xml_el: xml.etree.ElementTree.Element, cast=None) -> str: return xml_el.text -def _check_derived_image_xml(derived: xml.etree.ElementTree.Element): +def _check_xml_field( + xml_element: xml.etree.ElementTree.Element, field: str, expected_value: str +) -> None: + """Check that the given field of the given XML element has the expected value.""" + if (found_value := _check_xml_and_get_text(xml_element, field)) != expected_value: + raise ValueError( + f"The {field} for the derived image should be '{expected_value}'. " + f"{found_value} was found instead." + ) + + +def _check_derived_image_xml(derived: xml.etree.ElementTree.Element) -> None: """Perform sanity checks on derived images.""" - assert len(derived) >= 9 # multiple nodes possible - assert ( - _check_xml_and_get_text(derived[2], "imageType") == "image volume" - ), "imageType" - assert _check_xml_and_get_text(derived[3], "tissue") == "All", "tissue" - assert _check_xml_and_get_text(derived[4], "hemisphere") == "Both", "hemis" - assert ( - _check_xml_and_get_text(derived[5], "anatomicStructure") == "Brain" - ), "anat struct" - assert ( - _check_xml_and_get_text(derived[6], "registration") == "native" - ), "registration not native" - # skip check relatedImage, original ID consistence + "derived from"... - # skip processing steps with names, versions, dates and so on - # 2 cases - if derived[-1].tag == "creationDate": - assert ( - _check_xml_and_get_text(derived[-1], "creationDate") == "0000-00-00" - ), "creaDate" - else: - assert ( - _check_xml_and_get_text(derived[-2], "creationDate") == "0000-00-00" - ), "creaDate" + if len(derived) < 9: + raise ValueError("derived image does not have enough field") + for idx, field, expected in zip( + [2, 3, 4, 5, 6], + ["imageType", "tissue", "hemisphere", "anatomicStructure", "registration"], + ["image volume", "All", "Both", "Brain", "native"], + ): + _check_xml_field(derived[idx], field, expected) + idx = -1 if derived[-1].tag == "creationDate" else -2 + if ( + image_create_date := _check_xml_and_get_text(derived[idx], "creationDate") + ) != "0000-00-00": + try: + _validate_date_iso_format(image_create_date) + except ValueError as e: + raise ValueError( + f"The creationDate for the derived image is not valid: {e}" + ) + + +def _validate_date_iso_format(date: str) -> None: + import datetime + + try: + datetime.date.fromisoformat(date) + except ValueError: + raise ValueError(f"Incorrect data format {date}, should be YYYY-MM-DD") def _get_derived_image_metadata(derived: xml.etree.ElementTree.Element) -> dict: From 8f9c2e590330929539bb4064218ebdf88e44feff Mon Sep 17 00:00:00 2001 From: NicolasGensollen Date: Thu, 24 Aug 2023 14:00:04 +0200 Subject: [PATCH 2/2] move helper function definition after caller... --- .../converters/adni_to_bids/adni_json.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/clinica/iotools/converters/adni_to_bids/adni_json.py b/clinica/iotools/converters/adni_to_bids/adni_json.py index 59dde45bc..409a12d0b 100644 --- a/clinica/iotools/converters/adni_to_bids/adni_json.py +++ b/clinica/iotools/converters/adni_to_bids/adni_json.py @@ -102,17 +102,6 @@ def _get_text(xml_el: xml.etree.ElementTree.Element, cast=None) -> str: return xml_el.text -def _check_xml_field( - xml_element: xml.etree.ElementTree.Element, field: str, expected_value: str -) -> None: - """Check that the given field of the given XML element has the expected value.""" - if (found_value := _check_xml_and_get_text(xml_element, field)) != expected_value: - raise ValueError( - f"The {field} for the derived image should be '{expected_value}'. " - f"{found_value} was found instead." - ) - - def _check_derived_image_xml(derived: xml.etree.ElementTree.Element) -> None: """Perform sanity checks on derived images.""" if len(derived) < 9: @@ -135,6 +124,17 @@ def _check_derived_image_xml(derived: xml.etree.ElementTree.Element) -> None: ) +def _check_xml_field( + xml_element: xml.etree.ElementTree.Element, field: str, expected_value: str +) -> None: + """Check that the given field of the given XML element has the expected value.""" + if (found_value := _check_xml_and_get_text(xml_element, field)) != expected_value: + raise ValueError( + f"The {field} for the derived image should be '{expected_value}'. " + f"{found_value} was found instead." + ) + + def _validate_date_iso_format(date: str) -> None: import datetime