diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7d2dc3e0aad989eef459b1ea8806a564166cefdf..7786c99562ec215967f5757986a1ebb6a526603d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added ###
### Changed ###
+- Renamed `_Parents` to `_ParentList`.
### Deprecated ###
@@ -17,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ###
+* [caosdb-pylib#90](https://gitlab.com/caosdb/caosdb-pylib/-/issues/90): `Entity.get_parents_recursively()` did not work for unretrieved parents.
+
### Security ###
### Documentation ###
diff --git a/src/caosdb/common/models.py b/src/caosdb/common/models.py
index 83359ac847fa62de976208f9af023a2cf2a73af6..08fcd0206b9df22902e80277d0e57b5f67c76db5 100644
--- a/src/caosdb/common/models.py
+++ b/src/caosdb/common/models.py
@@ -34,6 +34,7 @@ All additional classes are either important for the entities or the
transactions.
"""
from __future__ import print_function, unicode_literals
+from __future__ import annotations # Can be removed with 3.10.
import re
import sys
@@ -82,7 +83,7 @@ SPECIAL_ATTRIBUTES = ["name", "role", "datatype", "description",
"id", "path", "checksum", "size"]
-class Entity(object):
+class Entity:
"""Entity is a generic CaosDB object.
@@ -101,6 +102,8 @@ class Entity(object):
self._checksum = None
self._size = None
self._upload = None
+ # If an entity is used (e.g. as parent), it is wrapped instead of being used directly.
+ # see Entity._wrap()
self._wrapped_entity = None
self._version = None
self._cuid = None
@@ -111,7 +114,7 @@ class Entity(object):
self.value = value
self.messages = _Messages()
self.properties = _Properties()
- self.parents = _Parents()
+ self.parents = _ParentList()
self.path = None
self.file = None
self.unit = None
@@ -530,7 +533,7 @@ class Entity(object):
value=value, unit=unit)
if abstract_property is not None:
- new_property._wrap(property)
+ new_property._wrap(abstract_property)
# FIXME: this really necessary?
@@ -621,26 +624,46 @@ class Entity(object):
return self
- def has_parent(self, parent, recursive=True,
- check_name=True, check_id=False):
- """Checks if this entity has a given parent.
+ def has_parent(self, parent: Entity, recursive: bool = True, retrieve: bool = True,
+ check_name: bool = True, check_id: bool = False):
+ """Check if this entity has a given parent.
If 'check_name' and 'check_id' are both False, test for identity
on the Python level. Otherwise use the name and/or ID for the
check. Note that, if checked, name or ID should not be None,
lest the check fail.
- @param parent: Check for this parent.
- @param recursive: Whether to check recursively.
- @param check_name: Whether to use the name for ancestry check.
- @param check_id: Whether to use the ID for ancestry check.
- @return: True if 'parent' is a true parent, False otherwise.
- """
+Parameters
+----------
+
+parent: Entity
+ Check for this parent.
+
+recursive: bool, optional
+ Whether to check recursively.
+
+check_name: bool, optional
+ Whether to use the name for ancestry check.
+
+check_id: bool, optional
+ Whether to use the ID for ancestry check.
+
+retrieve: bool, optional
+ If False, do not retrieve parents from the server.
+
+Returns
+-------
+out: bool
+ True if ``parent`` is a true parent, False otherwise.
+"""
if recursive:
- parents = self.get_parents_recursively()
+ parents = self.get_parents_recursively(retrieve=retrieve)
else:
- parents = [pp._wrapped_entity for pp in self.parents]
+ if retrieve:
+ parents = [pp.retrieve()._wrapped_entity for pp in self.parents]
+ else:
+ parents = [pp._wrapped_entity for pp in self.parents]
if not (check_name or check_id):
return parent in parents
@@ -659,39 +682,61 @@ class Entity(object):
def get_parents(self):
"""Get all parents of this entity.
- @return: _Parents(list)
+ @return: _ParentList(list)
"""
return self.parents
- def get_parents_recursively(self):
+ def get_parents_recursively(self, retrieve: bool = True):
"""Get all ancestors of this entity.
- @return: list of Entities
- """
+Parameters
+----------
+
+retrieve: bool, optional
+ If False, do not retrieve parents from the server.
+
+Returns
+-------
+out: List[Entity]
+ The parents of this Entity
+"""
- all_parents = _Parents()
- self._get_parent_recursively(all_parents)
+ all_parents = []
+ self._get_parent_recursively(all_parents, retrieve=retrieve)
return all_parents
- def _get_parent_recursively(self, all_parents):
+ def _get_parent_recursively(self, all_parents: list, retrieve: bool = True):
"""Get all ancestors with a little helper.
As a side effect of this method, the ancestors are added to
all_parents.
- @param all_parents: The added parents so far.
+ @param all_parents: list, The added parents so far.
@return: None, but see side effects.
"""
for parent in self.parents:
+ # TODO:
+ # Comment on _wrap and _wrapped_entity
+ # Currently, I (henrik) do not why the wrapping is necessary (and it is not
+ # documented). However, the following illustrates, why I think, it is a bad idea.
+ # First you add a parent with rec.add_parent(parent), but then you cannot access
+ # attributes of parent when you use rec.parents[0] for example becasue you do not get
+ # the same object but a wrapping object and you need to know that you only get the
+ # original by accessing the private (!) _wrapped_entity object.
w_parent = parent._wrapped_entity
+ if retrieve:
+ parent.retrieve()
+ for next_parent in parent.parents:
+ w_parent.add_parent(next_parent)
- if w_parent not in all_parents:
+ if (w_parent.id, w_parent.name) not in [
+ (all_p.id, all_p.name) for all_p in all_parents]:
all_parents.append(w_parent)
- w_parent._get_parent_recursively(all_parents)
+ w_parent._get_parent_recursively(all_parents, retrieve=retrieve)
def get_parent(self, key):
"""Return the first parent matching the key or None if no match exists.
@@ -1135,7 +1180,7 @@ class Entity(object):
else:
raise TypeError(
'Child was neither a Property, nor a Parent, nor a Message.\
- Was ' + str(type(child)))
+ Was ' + str(type(child)) + "\n" + str(child))
# add VALUE
value = None
@@ -1296,6 +1341,12 @@ class Entity(object):
flags=flags)[0]
def _wrap(self, entity):
+ """
+ When entity shall be used as parent or property it is not added to the corresponding list
+ (such as the parent list) directly, but another Entity object is created and the original
+ Entity is wrapped using this function
+ TODO: document here and in dev docs why this is done.
+ """
self._wrapped_entity = entity
return self
@@ -2094,7 +2145,8 @@ class _Properties(list):
raise KeyError(str(prop) + " not found.")
-class _Parents(list):
+class _ParentList(list):
+ # TODO unclear why this class is private. Isn't it use full for users?
def _get_entity_by_cuid(self, cuid):
'''
@@ -2697,9 +2749,11 @@ class Container(list):
elif isinstance(entity, QueryTemplate):
super().append(entity)
else:
- raise TypeError(
- "Entity was neither an id nor a name nor an entity." +
- " (was " + str(type(entity)) + ")")
+ warn("Entity was neither an id nor a name nor an entity." +
+ " (was " + str(type(entity)) + ":\n" + str(entity) + ")")
+ # raise TypeError(
+ # "Entity was neither an id nor a name nor an entity." +
+ # " (was " + str(type(entity)) + "\n" + str(entity) + ")")
return self
@@ -3647,6 +3701,7 @@ class Container(list):
for p in e.get_properties():
if p.id is None:
if p.name is not None:
+ # TODO using try except for normal execution flow is bad style
try:
w = self.get_entity_by_name(p.name)
p._wrap(w)
@@ -3658,6 +3713,7 @@ class Container(list):
for p in e.get_parents():
if p.id is None:
if p.name is not None:
+ # TODO using try except for normal execution flow is bad style
try:
p._wrap(self.get_entity_by_name(p.name))
except KeyError: