diff --git a/.github/workflows/plugin_test.yaml b/.github/workflows/plugin_test.yaml
index 5a8654912..07c5ed3c6 100644
--- a/.github/workflows/plugin_test.yaml
+++ b/.github/workflows/plugin_test.yaml
@@ -36,10 +36,10 @@ jobs:
branch: main
tests_to_run: tests/.
- plugin: pynxtools-raman
- branch: main
+ branch: sibling-inheritance
tests_to_run: tests/.
- plugin: pynxtools-spm
- branch: main
+ branch: field-inheritance
tests_to_run: tests/.
- plugin: pynxtools-xps
branch: main
diff --git a/src/pynxtools/data/NXtest.nxdl.xml b/src/pynxtools/data/NXtest.nxdl.xml
index 329f42ef2..46035acfd 100644
--- a/src/pynxtools/data/NXtest.nxdl.xml
+++ b/src/pynxtools/data/NXtest.nxdl.xml
@@ -123,6 +123,7 @@
+
diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py
index f47b171f4..d9664a967 100644
--- a/src/pynxtools/dataconverter/helpers.py
+++ b/src/pynxtools/dataconverter/helpers.py
@@ -24,7 +24,7 @@
from datetime import datetime, timezone
from enum import Enum
from functools import lru_cache
-from typing import Any, Callable, List, Optional, Tuple, Union, Sequence
+from typing import Any, Callable, List, Optional, Tuple, Union, Sequence, cast
import h5py
import lxml.etree as ET
@@ -67,6 +67,7 @@ class ValidationProblem(Enum):
NXdataMissingAxisData = 19
NXdataAxisMismatch = 20
KeyToBeRemoved = 21
+ InvalidConceptForNonVariadic = 22
class Collector:
@@ -150,6 +151,12 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar
)
elif log_type == ValidationProblem.KeyToBeRemoved:
logger.warning(f"The attribute {path} will not be written.")
+ elif log_type == ValidationProblem.InvalidConceptForNonVariadic:
+ value = cast(Any, value)
+ log_text = f"Given {value.type} name '{path}' conflicts with the non-variadic name '{value}'"
+ if value.type == "group":
+ log_text += f", which should be of type {value.nx_class}."
+ logger.warning(log_text)
def collect_and_log(
self,
diff --git a/src/pynxtools/dataconverter/nexus_tree.py b/src/pynxtools/dataconverter/nexus_tree.py
index 79ae5cf68..5d2a2a9d6 100644
--- a/src/pynxtools/dataconverter/nexus_tree.py
+++ b/src/pynxtools/dataconverter/nexus_tree.py
@@ -41,6 +41,7 @@
is_variadic,
is_appdef,
remove_namespace_from_tag,
+ NEXUS_TO_PYTHON_DATA_TYPES,
)
from pynxtools.definitions.dev_tools.utils.nxdl_utils import (
get_nx_namefit,
@@ -214,19 +215,6 @@ def __init__(
self.is_a = []
self.parent_of = []
- def _construct_inheritance_chain_from_parent(self):
- """
- Builds the inheritance chain of the current node based on the parent node.
- """
- if self.parent is None:
- return
- for xml_elem in self.parent.inheritance:
- elem = xml_elem.find(
- f"nx:{self.type}/[@name='{self.name}']", namespaces=namespaces
- )
- if elem is not None:
- self.inheritance.append(elem)
-
def get_path(self) -> str:
"""
Gets the path of the current node based on the node name.
@@ -356,6 +344,7 @@ def get_all_direct_children_names(
Returns:
Set[str]: A set of children names.
"""
+
if depth is not None and (not isinstance(depth, int) or depth < 0):
raise ValueError("Depth must be a positive integer or None")
@@ -601,6 +590,7 @@ def add_node_from(self, xml_elem: ET._Element) -> Optional["NexusNode"]:
type=tag,
optionality=default_optionality,
nxdl_base=xml_elem.base,
+ inheritance=[xml_elem],
)
elif tag == "group":
name = xml_elem.attrib.get("name")
@@ -684,6 +674,19 @@ def __init__(self, **data) -> None:
self._construct_inheritance_chain_from_parent()
self._set_optionality()
+ def _construct_inheritance_chain_from_parent(self):
+ """
+ Builds the inheritance chain of the current node based on the parent node.
+ """
+ if self.parent is None:
+ return
+ for xml_elem in self.parent.inheritance:
+ elem = xml_elem.find(
+ f"nx:{self.type}/[@name='{self.name}']", namespaces=namespaces
+ )
+ if elem is not None:
+ self.inheritance.append(elem)
+
class NexusGroup(NexusNode):
"""
@@ -864,6 +867,145 @@ class NexusEntity(NexusNode):
open_enum: bool = False
shape: Optional[Tuple[Optional[int], ...]] = None
+ def _check_compatibility_with(self, xml_elem: ET._Element) -> bool:
+ """Check compatibility of this node with an XML element from the (possible) inheritance"""
+
+ def _check_name_fit(xml_elem: ET._Element) -> bool:
+ elem_name = xml_elem.attrib.get("name")
+ name_any = is_name_type(xml_elem, "any")
+ name_partial = is_name_type(xml_elem, "partial")
+
+ if get_nx_namefit(self.name, elem_name, name_any, name_partial) < 0:
+ return False
+ return True
+
+ def _check_type_fit(xml_elem: ET._Element) -> bool:
+ elem_type = xml_elem.attrib.get("type")
+ if elem_type:
+ if not set(NEXUS_TO_PYTHON_DATA_TYPES[self.dtype]).issubset(
+ NEXUS_TO_PYTHON_DATA_TYPES[elem_type]
+ ):
+ return False
+ return True
+
+ def _check_units_fit(xml_elem: ET._Element) -> bool:
+ elem_units = xml_elem.attrib.get("units")
+ if elem_units and elem_units != "NX_ANY":
+ if elem_units != self.unit:
+ if not elem_units == "NX_TRANSFORMATION" and self.unit in [
+ "NX_LENGTH",
+ "NX_ANGLE",
+ "NX_UNITLESS",
+ ]:
+ return False
+ return True
+
+ def _check_enum_fit(xml_elem: ET._Element) -> bool:
+ elem_enum = xml_elem.find(f"nx:enumeration", namespaces=namespaces)
+ if elem_enum is not None:
+ if self.items is None:
+ # Case where inherited entity is enumerated, but current node isn't
+ return False
+ elem_enum_open = elem_enum.attrib.get("open", "false")
+
+ if elem_enum_open == "true":
+ return True
+
+ elem_enum_items = []
+ for items in elem_enum.findall(f"nx:item", namespaces=namespaces):
+ value = items.attrib["value"]
+ if value[0] == "[" and value[-1] == "]":
+ import ast
+
+ try:
+ elem_enum_items.append(ast.literal_eval(value))
+ except (ValueError, SyntaxError):
+ raise Exception(
+ f"Error parsing enumeration item in the provided NXDL: {value}"
+ )
+ else:
+ elem_enum_items.append(value)
+
+ def convert_to_hashable(item):
+ """Convert lists to tuples for hashable types, leave non-list items as they are."""
+ if isinstance(item, list):
+ return tuple(item) # Convert sublists to tuples
+ return item # Non-list items remain as they are
+
+ set_items = {convert_to_hashable(sublist) for sublist in self.items}
+ set_elem_enum_items = {
+ convert_to_hashable(sublist) for sublist in elem_enum_items
+ }
+
+ if not set(set_items).issubset(set_elem_enum_items):
+ if self.name == "definition":
+ pass
+ else:
+ # TODO: should we be this strict here? Or can appdefs define additional terms?
+ pass
+ return True
+
+ def _check_dimensions_fit(xml_elem: ET._Element) -> bool:
+ if not self.shape:
+ return True
+ elem_dimensions = xml_elem.find(f"nx:dimensions", namespaces=namespaces)
+ if elem_dimensions is not None:
+ rank = elem_dimensions.attrib.get("rank")
+ if rank is not None and not isinstance(rank, int):
+ try:
+ int(rank)
+ except ValueError:
+ # TODO: Handling of symbols
+ return True
+ elem_dim = elem_dimensions.findall("nx:dim", namespaces=namespaces)
+ elem_dimension_rank = rank if rank is not None else len(rank)
+ dims: List[Optional[int]] = [None] * int(rank)
+
+ for dim in elem_dim:
+ idx = int(dim.attrib["index"])
+ if value := dim.attrib.get("value", None):
+ # If not, this is probably an old dim element with ref.
+ try:
+ value = int(value)
+ dims[idx] = value
+ except ValueError:
+ # TODO: Handling of symbols
+ pass
+ elem_shape = tuple(dims)
+
+ if elem_shape:
+ if elem_shape != self.shape:
+ return False
+
+ return True
+
+ check_functions = [
+ _check_name_fit,
+ _check_type_fit,
+ _check_units_fit,
+ _check_enum_fit,
+ # TODO: check if any inheritance is wrongfully assigned without dim checks
+ # _check_dimensions_fit,
+ ]
+
+ for func in check_functions:
+ if not func(xml_elem):
+ return False
+ return True
+
+ def _construct_inheritance_chain_from_parent(self):
+ """
+ Builds the inheritance chain of the current node based on the parent node.
+ """
+ if self.parent is None:
+ return
+ for xml_elem in self.parent.inheritance:
+ subelems = xml_elem.findall(f"nx:{self.type}", namespaces=namespaces)
+ if subelems is not None:
+ for elem in subelems:
+ if self._check_compatibility_with(elem):
+ self.inheritance.append(elem)
+
def _set_type(self):
"""
Sets the dtype of the current entity based on the values in the inheritance chain.
@@ -950,7 +1092,13 @@ def _set_shape(self):
def __init__(self, **data) -> None:
super().__init__(**data)
+ self._set_unit()
+ self._set_type()
+ self._set_items_and_enum_type()
+ self._set_optionality()
+ self._set_shape()
self._construct_inheritance_chain_from_parent()
+ # Set all parameters again based on the acquired inheritance
self._set_unit()
self._set_type()
self._set_items_and_enum_type()
diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py
index 098f0a50f..d0bd645e2 100644
--- a/src/pynxtools/dataconverter/validation.py
+++ b/src/pynxtools/dataconverter/validation.py
@@ -155,6 +155,31 @@ def best_namefit_of(name: str, nodes: Iterable[NexusNode]) -> Optional[NexusNode
for node in nodes:
if not node.variadic:
if instance_name == node.name:
+ if concept_name and concept_name != node.name:
+ inherited_names = [
+ name
+ if (name := elem.attrib.get("name")) is not None
+ else type_attr[2:].upper()
+ for elem in node.inheritance
+ if (name := elem.attrib.get("name")) is not None
+ or (type_attr := elem.attrib.get("type"))
+ and len(type_attr) > 2
+ ]
+ if concept_name not in inherited_names:
+ if node.type == "group":
+ if concept_name != node.nx_class[2:].upper():
+ collector.collect_and_log(
+ concept_name,
+ ValidationProblem.InvalidConceptForNonVariadic,
+ node,
+ )
+ else:
+ collector.collect_and_log(
+ concept_name,
+ ValidationProblem.InvalidConceptForNonVariadic,
+ node,
+ )
+ return None
return node
else:
if concept_name and concept_name == node.name:
@@ -194,16 +219,32 @@ def validate_dict_against(
"""
def get_variations_of(node: NexusNode, keys: Mapping[str, Any]) -> List[str]:
+ variations = []
+
+ prefix = f"{'@' if node.type == 'attribute' else ''}"
if not node.variadic:
- if f"{'@' if node.type == 'attribute' else ''}{node.name}" in keys:
- return [node.name]
+ if f"{prefix}{node.name}" in keys:
+ variations += [node.name]
elif (
hasattr(node, "nx_class")
and f"{convert_nexus_to_caps(node.nx_class)}[{node.name}]" in keys
):
- return [f"{convert_nexus_to_caps(node.nx_class)}[{node.name}]"]
-
- variations = []
+ variations += [f"{convert_nexus_to_caps(node.nx_class)}[{node.name}]"]
+
+ # Also add all variations like CONCEPT[node.name] for inherited concepts
+ inherited_names = []
+ for elem in node.inheritance:
+ inherited_name = elem.attrib.get("name")
+ if not inherited_name:
+ inherited_name = elem.attrib.get("type")[2:].upper()
+ if inherited_name.startswith("NX"):
+ inherited_name = inherited_name[2:].upper()
+ inherited_names += [inherited_name]
+ for name in set(inherited_names):
+ if f"{prefix}{name}[{prefix}{node.name}]" in keys:
+ variations += [f"{prefix}{name}[{prefix}{node.name}]"]
+
+ return variations
for key in keys:
concept_name, instance_name = split_class_and_name_of(key)
diff --git a/tests/dataconverter/test_validation.py b/tests/dataconverter/test_validation.py
index 80cbbb9b1..67eaa4ef2 100644
--- a/tests/dataconverter/test_validation.py
+++ b/tests/dataconverter/test_validation.py
@@ -722,20 +722,55 @@ def listify_template(data_dict: Template):
pytest.param(
alter_dict(
remove_from_dict(
- TEMPLATE,
- "/ENTRY[my_entry]/optional_parent/required_child",
+ remove_from_dict(
+ remove_from_dict(
+ TEMPLATE,
+ "/ENTRY[my_entry]/specified_group/specified_field",
+ "required",
+ ),
+ "/ENTRY[my_entry]/specified_group/specified_field/@specified_attr_in_field",
+ "required",
+ ),
+ "/ENTRY[my_entry]/specified_group/@specified_attr",
"required",
),
- "/ENTRY[my_entry]/optional_parent/AXISNAME[required_child]",
- 1,
+ "/ENTRY[my_entry]/SAMPLE[specified_group]/specified_field",
+ 1.0,
),
- # ToDo: should not raise a warning if sibling inheritance works
[
- "The data entry corresponding to /ENTRY[my_entry]/optional_parent/"
- "required_child is required and hasn't been supplied by the reader."
+ "The required group, /ENTRY[my_entry]/specified_group, hasn't been supplied.",
+ "Given group name 'SAMPLE' conflicts with the non-variadic name 'specified_group (req)', "
+ "which should be of type NXdata.",
+ "Field /ENTRY[my_entry]/SAMPLE[specified_group]/specified_field written without documentation.",
],
+ id="illegal-concept-name-for-nonvariadic-group",
+ ),
+ pytest.param(
+ alter_dict(
+ alter_dict(
+ remove_from_dict(
+ TEMPLATE,
+ "/ENTRY[my_entry]/optional_parent/required_child",
+ "required",
+ ),
+ "/ENTRY[my_entry]/optional_parent/AXISNAME[required_child]",
+ 1,
+ ),
+ "/ENTRY[my_entry]/optional_parent/AXISNAME[optional_child]",
+ 1,
+ ),
+ [],
id="concept-name-given-for-nonvariadic-field",
),
+ pytest.param(
+ alter_dict(
+ TEMPLATE,
+ "/ENTRY[my_entry]/optional_parent/@AXISNAME_indices[@required_child_indices]",
+ 0,
+ ),
+ [],
+ id="concept-name-given-for-optional-attribute",
+ ),
pytest.param(
alter_dict(
remove_from_dict(
@@ -1053,22 +1088,21 @@ def listify_template(data_dict: Template):
"123",
),
[],
+ id="specified-identifier-without-type",
+ ),
+ pytest.param(
+ alter_dict(
+ alter_dict(
+ TEMPLATE,
+ "/ENTRY[my_entry]/identified_calibration/identifier_1",
+ "123",
+ ),
+ "/ENTRY[my_entry]/identified_calibration/identifier_1/@type",
+ "ORCID",
+ ),
+ [],
id="specified-identifier-with-type",
),
- # ToDo: reactivate if sibling inheritance works properly
- # pytest.param(
- # alter_dict(
- # alter_dict(
- # TEMPLATE,
- # "/ENTRY[my_entry]/identified_calibration/identifier_1",
- # "123",
- # ),
- # "/ENTRY[my_entry]/identified_calibration/identifier_1/@type",
- # "ORCID",
- # ),
- # [],
- # id="specified-identifier-with-type",
- # ),
pytest.param(
alter_dict(
alter_dict(
@@ -1095,6 +1129,31 @@ def listify_template(data_dict: Template):
[],
id="name-fitted-identifier-with-type",
),
+ pytest.param(
+ alter_dict(
+ TEMPLATE,
+ "/ENTRY[my_entry]/CALIBRATION[identified_calibration]/identifier_1",
+ "123",
+ ),
+ [],
+ id="group-with-correct-concept",
+ ),
+ pytest.param(
+ alter_dict(
+ alter_dict(
+ TEMPLATE,
+ "/ENTRY[my_entry]/CALIBRATION[identified_calibration]/identifier_1",
+ "123",
+ ),
+ "/ENTRY[my_entry]/identified_calibration/identifier_2",
+ "456",
+ ),
+ [
+ "The data entry corresponding to /ENTRY[my_entry]/identified_calibration/identifier_1 is required "
+ "and hasn't been supplied by the reader."
+ ],
+ id="group-with-correct-concept-and-non-concept-sibling",
+ ),
# This can be re-used later when we have proper unit checking
pytest.param(
alter_dict(