emtpb / dsch

Structured, metadata-enhanced data storage.
Other
2 stars 0 forks source link

depends_on property fails in nested List+Array #3

Open JonasHoelscher opened 5 months ago

JonasHoelscher commented 5 months ago

If the dsch schema looks like this

Comp(
   {
      "channel_1": Lt(Ar(
         dtype='float', unit='V', depends_on='channel_2')),
      "channel_2": Lt(Ar(
          dtype='float', unit='V', depends_on='channel_1')),
   }
)

When running the save() function an exception is thrown: AttributeError: 'List' object has no attribute 'channel_2'

This error does not occur when the validation is skipped: By using save(force=True).

Possible cause: The problem is that the validation does only look at the parent node and when the Array inside of the List has a depends_on property it will check the parent node, in this case the list, if it has an item called "channel_2".

This can be seen in dsch/data.py:

def validate(self):
        """Validate the node value against the schema node specification.

        If validation succeeds, the method terminates silently. Otherwise, an
        exception is raised.

        Raises:
            dsch.exceptions.ValidationError: if validation fails.
        """
        if self._storage is None:
            return
        independent_values = []
        node_names = self.schema_node.depends_on or []
        for node_name in node_names:
            if node_name:
                independent_values.append(getattr(self.parent, node_name)
                                          .value)
        self.schema_node.validate(self.value, independent_values)

Where when the items are added to the independent_values list only the direct parent is taken.

polymeter commented 5 months ago

Your intended schema has two issues that lead to this error: 1) Depending on a list of arrays is ambiguous: Which element of the list will depend on which element of the other list? 2) Dependencies can only be one-way from dependent to independent variable.

I agree that we could provide a more helpful error message here, but the limitations are intended.

lndr commented 5 months ago
  1. I'd suggest we raise the error in the schema definition if depends_on is of an unsupported class?
  2. We could implement a similar behaviour here, too, but checking for circular references (right?) might be a tad more complex to implement efficiently.