Improve error catching and message for add

docker/irods_client/tests/test_data_ops.py

@@ -135,7 +135,7 @@ def test_meta_archive(session, testdata, tmpdir):
     sync(session, testdata, ipath)
     assert len(list(ipath.meta)) == 0
     meta_list = [
-        (ipath, ("root", "true", None)),
+        (ipath, ("root", "true", "")),
         (ipath / "more_data", ("more_data", "false", "kg")),
         (ipath / "more_data" / "polarbear.txt", ("is_polar", "true", "bool")),
     ]

docker/irods_client/tests/test_meta.py

@@ -23,7 +23,7 @@ def test_meta(item_name, request):
     assert len(meta) == 1
     assert list(meta)[0].name == "x"
     assert list(meta)[0].value == "y"
-    assert list(meta)[0].units is None
+    assert list(meta)[0].units == ""
     assert "x" in meta
     assert ("x", "y") in meta
     assert "y" not in meta

ibridges/meta.py

@@ -11,6 +11,18 @@
 import irods.meta
 
 
+def _parse_tuple(key, value, units = ""):
+    if key == "":
+        raise ValueError("Key cannot be of size zero.")
+    if not isinstance(key, (str, bytes)):
+        raise TypeError(f"Key should have type str or bytes-like, " f"not {type(key)}.")
+    if value == "":
+        raise ValueError("Value cannot be of size zero.")
+    if not isinstance(value, (str, bytes)):
+        raise TypeError(f"Value should have type str or bytes-like, " f"not {type(value)}.")
+    if not isinstance(units, (str, bytes, type(None))):
+        raise TypeError(f"Key should have type str, bytes-like or None, " f"not {type(units)}.")
+
 class MetaData:
     """iRODS metadata operations.
 
@@ -60,8 +72,10 @@ def __iter__(self) -> Iterator:
             if not self.blacklist or re.match(self.blacklist, meta.name) is None:
                 yield MetaDataItem(self, meta)
             else:
-                warnings.warn(f"Ignoring metadata entry with key {meta.name}, because it matches "
-                              f"the blacklist {self.blacklist}.")
+                warnings.warn(
+                    f"Ignoring metadata entry with key {meta.name}, because it matches "
+                    f"the blacklist {self.blacklist}."
+                )
 
     def __len__(self) -> int:
         """Get the number of non-blacklisted metadata entries."""
@@ -89,7 +103,6 @@ def __contains__(self, val: Union[str, Sequence]) -> bool:
             return True
         return False
 
-
     def __repr__(self) -> str:
         """Create a sorted representation of the metadata."""
         return f"MetaData<{self.item.path}>"
@@ -100,7 +113,7 @@ def __str__(self) -> str:
         meta_list = sorted(list(self))
         return "\n".join(f" - {meta}" for meta in meta_list)
 
-    def find_all(self, key = ..., value = ..., units = ...):
+    def find_all(self, key=..., value=..., units=...):
         """Find all metadata entries belonging to the data object/collection.
 
         Wildcards can be used by leaving the key/value/units at default.
@@ -136,10 +149,12 @@ def __getitem__(self, key: Union[str, Sequence[Union[str, None]]]) -> MetaDataIt
         search_pattern = _pad_search_pattern(key)
         all_items = self.find_all(*search_pattern)
         if len(all_items) == 0:
-            raise KeyError(f"Cannot find metadata item with '{key}'")
+            raise KeyError(f"Cannot find metadata item with key '{key}'.")
         if len(all_items) > 1:
-            raise ValueError(f"Found multiple items with key '{key}', specify value and "
-                             "units as well, for example: meta[key, value, units].")
+            raise ValueError(
+                f"Found multiple items with key '{key}', specify value and "
+                "units as well, for example: meta[key, value, units]."
+            )
         return all_items[0]
 
     def __setitem__(self, key: Union[str, Sequence[Union[str, None]]], other: Sequence[str]):
@@ -166,13 +181,14 @@ def __setitem__(self, key: Union[str, Sequence[Union[str, None]]], other: Sequen
 
         """
         if isinstance(other, str):
-            raise TypeError("Cannot set the metadata item to a single string value. "
-                            f"Use meta[{key}].key = \"{other}\" to change only the key "
-                            "for example.")
+            raise TypeError(
+                "Cannot set the metadata item to a single string value. "
+                f'Use meta[{key}].key = "{other}" to change only the key '
+                "for example."
+            )
         self[key].update(*other)
 
-
-    def add(self, key: str, value: str, units: Optional[str] = None):
+    def add(self, key: str, value: str, units: Optional[str] = ""):
         """Add metadata to an item.
 
         This will never overwrite an existing entry. If the triplet already exists
@@ -203,29 +219,22 @@ def add(self, key: str, value: str, units: Optional[str] = None):
         >>> meta.add("Mass", "10", "kg")
 
         """
+        _parse_tuple(key, value, units)
         try:
             if (key, value, units) in self:
                 raise ValueError("ADD META: Metadata already present")
             if self.blacklist:
-                if re.match(self.blacklist, key):
-                    raise ValueError(f"ADD META: Key must not start with {self.blacklist}.")
+                try:
+                    if re.match(self.blacklist, key):
+                        raise ValueError(f"ADD META: Key must not start with {self.blacklist}.")
+                except TypeError as error:
+                    raise TypeError(
+                            f"Key {key} must be of type string, found {type(key)}") from error
             self.item.metadata.add(key, value, units)
         except irods.exception.CAT_NO_ACCESS_PERMISSION as error:
             raise PermissionError("UPDATE META: no permissions") from error
-        except irods.message.Bad_AVU_Field as error:
-            if key == "":
-                raise ValueError("Key cannot be of size zero.") from error
-            if value == "":
-                raise ValueError("Value cannot be of size zero.") from error
-            if not isinstance(value, (str, bytes)):
-                raise TypeError(f"Value should have type str or bytes-like, "
-                                f"not {type(value)}.") from error
-            if not isinstance(units, (str, bytes)):
-                raise TypeError(f"Units should have type str or bytes-like, "
-                                f"not {type(value)}.") from error
-            raise error
-
-    def set(self, key: str, value: str, units: Optional[str] = None):
+
+    def set(self, key: str, value: str, units: Optional[str] = ""):
         """Set the metadata entry.
 
         If the metadata entry already exists, then all metadata entries with
@@ -256,8 +265,11 @@ def set(self, key: str, value: str, units: Optional[str] = None):
         self.delete(key)
         self.add(key, value, units)
 
-    def delete(self, key: str, value: Union[None, str] = ...,  # type: ignore
-               units: Union[None, str] = ...):  # type: ignore
+    def delete(
+        self,
+        key: str,
+        value: Union[None, str] = ...,  # type: ignore
+        units: Union[None, str] = ...,):  # type: ignore
         """Delete a metadata entry of an item.
 
         Parameters
@@ -290,8 +302,10 @@ def delete(self, key: str, value: Union[None, str] = ...,  # type: ignore
         """
         all_meta_items = self.find_all(key, value, units)
         if len(all_meta_items) == 0:
-            raise KeyError(f"Cannot delete items with key={key}, value={value} and units={units}, "
-                           "since no metadata entries exist with those values.")
+            raise KeyError(
+                f"Cannot delete items with key={key}, value={value} and units={units}, "
+                "since no metadata entries exist with those values."
+            )
         for meta_item in all_meta_items:
             meta_item.remove()
 
@@ -385,7 +399,8 @@ def from_dict(self, meta_dict: dict):
             except ValueError:
                 pass
 
-class MetaDataItem():
+
+class MetaDataItem:
     """Interface for metadata entries.
 
     This is a substitute of the python-irodsclient iRODSMeta object.
@@ -434,9 +449,9 @@ def value(self, new_value: Optional[str]):
         self.update(*new_item_values)
 
     @property
-    def units(self) -> Optional[str]:
+    def units(self) -> str:
         """Return the units of the metadata item."""
-        return self._prc_meta.units
+        return "" if self._prc_meta.units is None else self._prc_meta.units
 
     @units.setter
     def units(self, new_units: Optional[str]):
@@ -451,15 +466,16 @@ def __repr__(self) -> str:
 
     def __str__(self) -> str:
         """User readable representation of MetaDataItem."""
-        return f"(key: {self.key}, value: {self.value}, units: {self.units})"
+        units = "" if self.units is None else self.units
+        return f"(key: '{self.key}', value: '{self.value}', units: '{units}')"
 
     def __iter__(self) -> Iterator[Optional[str]]:
         """Allow iteration over key, value, units."""
         yield self.key
         yield self.value
         yield self.units
 
-    def update(self, new_key: str, new_value: str, new_units: Optional[str] = None):
+    def update(self, new_key: str, new_value: str, new_units: Optional[str] = ""):
         """Update the metadata item changing the key/value/units.
 
         Parameters
@@ -479,13 +495,16 @@ def update(self, new_key: str, new_value: str, new_units: Optional[str] = None):
 
         """
         new_item_key = (new_key, new_value, new_units)
+        print(new_item_key)
         try:
             _new_item = self._ibridges_meta[new_item_key]
         except KeyError:
-            self._ibridges_meta.add(*new_item_key)
             try:
+                self._ibridges_meta.add(*new_item_key)
                 self._ibridges_meta.item.metadata.remove(self._prc_meta)
             # If we get an error, roll back the added metadata
+            except TypeError as error:
+                raise TypeError(repr(error)) from error
             except irods.exception.CAT_NO_ACCESS_PERMISSION as error:
                 self._ibridges_meta.delete(*new_item_key)
                 raise ValueError(
@@ -494,8 +513,10 @@ def update(self, new_key: str, new_value: str, new_units: Optional[str] = None):
                 ) from error
             self._prc_meta = self._ibridges_meta[new_item_key]._prc_meta  # pylint: disable=protected-access
         else:
-            raise ValueError(f"Cannot change key/value/units to '{new_item_key}' metadata item "
-                             "already exists.")
+            raise ValueError(
+                f"Cannot change key/value/units to '{new_item_key}' metadata item "
+                "already exists."
+            )
 
     def __getattribute__(self, attr: str):
         """Add name attribute and check if the metadata item is already removed."""
@@ -524,8 +545,7 @@ def remove(self):
     def __lt__(self, other: MetaDataItem) -> bool:
         """Compare two metadata items for sorting mainly."""
         if not isinstance(other, MetaDataItem):
-            raise TypeError(f"Comparison between MetaDataItem and {type(other)} "
-                            "not supported.")
+            raise TypeError(f"Comparison between MetaDataItem and {type(other)} " "not supported.")
         comp_key = _comp_str_none(self.key, other.key)
         if comp_key is not None:
             return comp_key
@@ -539,7 +559,7 @@ def __lt__(self, other: MetaDataItem) -> bool:
 
     def matches(self, key, value, units):
         """See whether the metadata item matches the key,value,units pattern."""
-        units = None if units == "" else units
+        units = "" if units is None else units
         if key is not ... and key != self.key:
             return False
         if value is not ... and value != self.value:
@@ -548,6 +568,7 @@ def matches(self, key, value, units):
             return False
         return True
 
+
 def _comp_str_none(obj: Optional[str], other: Optional[str]) -> Optional[bool]:
     if obj is None and other is not None:
         return True
@@ -557,15 +578,16 @@ def _comp_str_none(obj: Optional[str], other: Optional[str]) -> Optional[bool]:
         return None
     return str(obj) < str(other)
 
+
 def _pad_search_pattern(search_pattern) -> tuple:
     if isinstance(search_pattern, str):
         padded_pattern = (search_pattern, ..., ...)
     elif len(search_pattern) == 1:
-        padded_pattern = (*search_pattern, ..., ...)
+        padded_pattern = (*search_pattern, ..., ...) # type: ignore
     elif len(search_pattern) == 2:
-        padded_pattern = (*search_pattern, ...)
+        padded_pattern = (*search_pattern, ...) # type: ignore
     elif len(search_pattern) > 3:
         raise ValueError("Too many arguments for '[]', use key, value, units.")
     else:
-        padded_pattern = tuple(search_pattern)
+        padded_pattern = tuple(search_pattern) # type: ignore
     return padded_pattern

ibridges/search.py

@@ -139,7 +139,7 @@ def search_data(  # pylint: disable=too-many-branches
     path = IrodsPath(session, path)
 
     if metadata is None:
-        metadata = []
+        metadata = [] # type: ignore
     if isinstance(metadata, MetaSearch):
         metadata = [metadata]
 
@@ -178,7 +178,7 @@ def search_data(  # pylint: disable=too-many-branches
     if path_pattern is not None:
         _path_filter(path_pattern, queries)
 
-    for mf in metadata:
+    for mf in metadata: # type: ignore
         _meta_filter(mf, queries)
 
     if checksum is not None: