From 32c337d3facd601f9c63d519de4ee467dc1b5ff7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20tom=20W=C3=B6rden?= <h.tomwoerden@indiscale.com>
Date: Fri, 24 May 2024 11:29:03 +0200
Subject: [PATCH] MAINT: review corrections
---
src/caoscrawler/identifiable.py | 17 ++++-----
src/caoscrawler/identifiable_adapters.py | 2 +-
src/caoscrawler/sync_graph.py | 45 ++++++------------------
src/caoscrawler/sync_node.py | 14 ++------
4 files changed, 21 insertions(+), 57 deletions(-)
diff --git a/src/caoscrawler/identifiable.py b/src/caoscrawler/identifiable.py
index aead3769..d396fee5 100644
--- a/src/caoscrawler/identifiable.py
+++ b/src/caoscrawler/identifiable.py
@@ -84,9 +84,8 @@ class Identifiable():
def _value_representation(value) -> str:
"""returns the string representation of property values to be used in the hash function
- The string is the CaosDB ID of other Entities
- (Python Id only if there is no CaosDB ID) and the string representation of bool, float, int
- and str.
+ The string is the CaosDB ID in case of SyncNode objects (SyncNode objects must have an ID)
+ and the string representation of None, bool, float, int, datetime and str.
"""
if value is None:
@@ -95,7 +94,7 @@ class Identifiable():
if value.id is not None:
return str(value.id)
else:
- raise RuntimeError("Python Entity without id not allowed")
+ raise RuntimeError("Python Entity (SyncNode) without ID not allowed")
elif isinstance(value, list):
return "[" + ", ".join([Identifiable._value_representation(el) for el in value]) + "]"
elif (isinstance(value, str) or isinstance(value, int) or isinstance(value, float)
@@ -104,7 +103,7 @@ class Identifiable():
else:
raise ValueError(f"Unknown datatype of the value: {value}")
- @ staticmethod
+ @staticmethod
def _create_hashable_string(identifiable: Identifiable) -> str:
"""
creates a string from the attributes of an identifiable that can be hashed
@@ -121,13 +120,10 @@ class Identifiable():
return rec_string
def __eq__(self, other) -> bool:
- """
- Identifiables are equal if they belong to the same Record.
- equal if attribute representations are equal
- """
+ """ Identifiables are equal if they share the same ID or if the representation is equal """
if not isinstance(other, Identifiable):
raise ValueError("Identifiable can only be compared to other Identifiable objects.")
- elif self.record_id is not None and other.record_id is not None:
+ if self.record_id is not None and other.record_id is not None:
return self.record_id == other.record_id
elif self.get_representation() == other.get_representation():
return True
@@ -135,6 +131,7 @@ class Identifiable():
return False
def __repr__(self):
+ """ deterministic text representation of the identifiable """
pstring = json.dumps({k: str(v) for k, v in self.properties.items()})
return (f"{self.__class__.__name__} for RT {self.record_type}: id={self.record_id}; "
f"name={self.name}\n"
diff --git a/src/caoscrawler/identifiable_adapters.py b/src/caoscrawler/identifiable_adapters.py
index 9fa297cc..89d93847 100644
--- a/src/caoscrawler/identifiable_adapters.py
+++ b/src/caoscrawler/identifiable_adapters.py
@@ -298,7 +298,7 @@ startswith: bool, optional
name = None
if se.registered_identifiable is None:
- raise ValueError("no register_identifiable")
+ raise ValueError("no registered_identifiable")
# fill the values:
for prop in se.registered_identifiable.properties:
diff --git a/src/caoscrawler/sync_graph.py b/src/caoscrawler/sync_graph.py
index 3fd06833..738b0773 100644
--- a/src/caoscrawler/sync_graph.py
+++ b/src/caoscrawler/sync_graph.py
@@ -43,25 +43,21 @@ from .sync_node import SyncNode, TempID
logger = logging.getLogger(__name__)
-def _for_each_scalar_value(node: SyncNode, condition: Callable[[Any], bool], kind: str,
- value: Callable[[Any], Any] = None):
- """ helper function that performs an action on each value element of each property of a node
+def _set_each_scalar_value(node: SyncNode, condition: Callable[[Any], bool], value: Any):
+ """ helper function that conditionally replaces each value element of each property of a node
- The argument "kind" determines which action is performed on each property value:
- The action (remove or set) is performed on each property value of each property: in case of
- lists, it is performed on each list element. The action is only performed if the condition that
+ If the property value is a list, the replacement is done for each list entry.
+ The replacement is only performed if the condition that
is provided is fulfilled, i.e. the callable ``condition`` returns True. The callable
``condition`` must take the property value (or list element) as the sole argument.
- Thus, with "remove" you can conditionally remove values and with "set" you can conditionally
- replace values.
-
Args:
node (SyncNode): The node which provides the properties (and their values) to operate on.
- condition (Callable): A function with one argument which is interpreted as a condition: Only if
- it returns True for the property value, the action is executed.
- kind (str): Either "remove" or "set" depending on whether you want to remove the property or replace the property.
- value (Callable): A function returning a new value that is set as the property value. This function receives the old value as the single argument.
+ condition (Callable): A function with one argument which is interpreted as a condition:
+ Only if it returns True for the property value, the action is
+ executed.
+ value (Callable): A function returning a new value that is set as the property value. This
+ function receives the old value as the single argument.
Last review by Alexander Schlemmer on 2024-05-24.
"""
@@ -69,25 +65,9 @@ def _for_each_scalar_value(node: SyncNode, condition: Callable[[Any], bool], kin
if isinstance(p.value, list):
for ii, el in enumerate(p.value):
if condition(el):
- if kind == "remove":
- p.value.remove(el)
- elif kind == "set":
- p.value[ii] = value(el)
+ p.value[ii] = value(el)
elif condition(p.value):
- if kind == "remove":
- node.properties.remove(p)
- elif kind == "set":
- p.value = value(p.value)
-
-
-def _remove_each_scalar_value(node: SyncNode, condition: Callable[[Any], bool]):
- """ "remove" version of _for_each_scalar_value """
- _for_each_scalar_value(node, condition, "remove")
-
-
-def _set_each_scalar_value(node: SyncNode, condition: Callable[[Any], bool], value: Any):
- """ "set" version of _for_each_scalar_value """
- _for_each_scalar_value(node, condition, "set", value=value)
+ p.value = value(p.value)
class SyncGraph():
@@ -652,6 +632,3 @@ class SyncGraph():
for other_node in self._get_nodes_whose_identity_relies_on(node):
if self._identifiable_is_needed(other_node):
self._set_identifiable_of_node(other_node)
-
- def __repr__(self):
- return f"SyncNode with ID={self.id}"
diff --git a/src/caoscrawler/sync_node.py b/src/caoscrawler/sync_node.py
index 26dbb411..80a7e4a1 100644
--- a/src/caoscrawler/sync_node.py
+++ b/src/caoscrawler/sync_node.py
@@ -167,7 +167,7 @@ class SyncNode:
if unequal:
logger.error(
"The Crawler is trying to create an entity,"
- " but there are have conflicting property values."
+ " but there are conflicting property values."
f"Problematic Property: {p.name}\n"
f"First value:\n{entval}\n"
f"Second value:\n{pval}\n"
@@ -180,11 +180,8 @@ class SyncNode:
return ent
def __repr__(self) -> str:
+ """ somewhat concise text representation of the SyncNode """
res = f"\n=====================================================\n{self.role}\n"
- if hasattr(self, "_metadata"):
- res += f"user: {self._metadata['user']}\n"
- res += f"json: {self._metadata['json']}\n"
- res += "---------------------------------------------------\n"
res += yaml.dump(
{
"id": self.id,
@@ -219,13 +216,6 @@ class SyncNode:
+ "=====================================================\n"
)
- def is_unidentifiable(self) -> bool:
- """returns whether this is an unidentifiable Node"""
- return (
- self.registered_identifiable is not None
- and self.registered_identifiable.get_property("no-ident") is not None
- )
-
def parent_in_list(parent: Parent, plist: _ParentList) -> bool:
"""helper function that checks whether a parent with the same name or ID is in the plist"""
--
GitLab