chores: apply suggestions given by Copilot in the review

Author: ecarrenolozano

ontograph/config/settings.py

@@ -15,7 +15,7 @@
 
 # Read package information from pyproject.toml
 _PYPROJECT_PATH = Path(__file__).parents[2] / 'pyproject.toml'
-with open(_PYPROJECT_PATH, 'r') as f:
+with open(_PYPROJECT_PATH) as f:
     _PYPROJECT = toml.load(f)
 
 # Extract package metadata

ontograph/loader.py

@@ -283,7 +283,9 @@ def _download_ontology(self, name_id: str, format: str) -> Path:
                 f'Download functionality not implemented: {err}'
             ) from err
         except Exception as err:
-            logger.exception(f'Error downloading ontology {name_id} in format {format}: {err}')
+            logger.exception(
+                f'Error downloading ontology {name_id} in format {format}: {err}'
+            )
             raise RuntimeError(
                 f'Failed to download ontology {name_id} in format {format}: {err}'
             ) from err

ontograph/models.py

@@ -180,7 +180,7 @@ def get_download_url(
             ValueError: If the ontology or the specified format is not found in the catalog.
         """
         metadata = self.get_ontology_metadata(ontology_id, show_metadata=False)
-        if not metadata:
+        if metadata is None:
             raise ValueError(
                 f"No metadata found for ontology ID '{ontology_id}'."
             )

ontograph/queries/navigator.py

@@ -126,7 +126,7 @@ def get_ancestors(
             include_self (bool, optional): If True, include the term itself in the result. Defaults to False.
 
         Returns:
-            set[str]: A list of ancestor term IDs.
+            set[str]: A list of ancestor term IDs. None in case of having no ancestors.
 
         Raises:
             KeyError: If the term_id is not found in the ontology.
@@ -161,7 +161,7 @@ def get_ancestors_with_distance(
             include_self (bool): Whether to include the term itself at distance 0.
 
         Returns:
-            Iterator(tuple[Term, int]): Pairs of ancestor term objects and their distance (non-negative integer).
+            Iterator(tuple[Term, int]): Pairs of ancestor term objects and their distance (negative integer).
 
         Raises:
             KeyError: If the term is not found in the ontology.
@@ -171,7 +171,7 @@ def get_ancestors_with_distance(
             term = self.get_term(term_id)
         except KeyError:
             logger.exception(f"Term ID '{term_id}' not found in ontology.")
-            return iter([])
+            return iter(())
 
         visited = set()
         queue = deque()
@@ -246,7 +246,7 @@ def get_descendants_with_distance(
             include_self (bool): Whether to include the term itself at distance 0.
 
         Returns:
-            Iterator[tuple[Term, int]]: Generator of (descendant term, distance) pairs.
+            Iterator[tuple[Term, int]]: Generator of (descendant term, distance) pairs. Distance in positive numbers.
 
         Raises:
             KeyError: If the term ID is not found in the ontology.
@@ -255,7 +255,7 @@ def get_descendants_with_distance(
             term = self.get_term(term_id)
         except KeyError:
             logger.exception(f"Term ID '{term_id}' not found in ontology.")
-            return iter([])
+            return iter(())
 
         visited = set()
         queue = deque()

ontograph/queries/relations.py

@@ -70,11 +70,10 @@ def is_descendant(self, descendant_node: str, ancestor_node: str) -> bool:
             raise
 
     def is_sibling(self, node_a: str, node_b: str) -> bool:
-        """
-        Determine if two nodes are siblings (share at least one parent).
+        """Determine if two nodes are siblings (share at least one parent).
 
-        This method handles multiple inheritance scenarios, where nodes may have more than one parent.
         Siblings are defined as nodes that are not the same and share at least one parent (i.e., their sets of parents intersect).
+
         Args:
             node_a (str): The ID of the first node.
             node_b (str): The ID of the second node.
@@ -83,6 +82,7 @@ def is_sibling(self, node_a: str, node_b: str) -> bool:
             bool: True if the nodes are siblings (not the same node and share at least one parent), False otherwise.
 
         Raises:
+            KeyError: If either node_a or node_b is not found in the ontology.
             Exception: If an error occurs during parent lookup.
         """
         try:

pyproject.toml

@@ -177,9 +177,7 @@ ignore = [
     "E303",  # Name: too-many-blank-lines
     "E501",  # Name: line too long
     "E731",  # Name: lambda-assignment
-    "E741",  # allow I, O, l as variable names -> I is the identity matrix
-    # pyupgrade (UP)
-    "UP015"  # Name: redundant-open-modes
+    "E741"  # allow I, O, l as variable names -> I is the identity matrix
 ]
 select = [
     "B",  # flake8-bugbear

tests/test_client.py

@@ -57,7 +57,7 @@ def test_load_catalog_and_list_ontologies(client_catalog):
     # Should load catalog and list available ontologies (may be empty if no catalog present)
     client_catalog.load_catalog(force_download=False)
     ontologies = client_catalog.list_available_ontologies()
-    print(ontologies)
+
     assert isinstance(ontologies, list)
     # No assertion on content, as catalog may be empty in test env
 

tests/test_loader.py

@@ -58,10 +58,11 @@ def test_load_from_file_value_error(pronto_loader, tmp_path, monkeypatch):
     file_path = tmp_path / 'bad.obo'
     file_path.write_text('bad content')
 
-    # Patch pronto.Ontology to raise ValueError
-    monkeypatch.setattr(
-        'pronto.Ontology', lambda fp: (_ for _ in ()).throw(ValueError('fail'))
-    )
+    # Define the error-raising function inside the test
+    def raise_value_error(fp):
+        raise ValueError('fail')
+
+    monkeypatch.setattr('pronto.Ontology', raise_value_error)
     with pytest.raises(ValueError):
         pronto_loader.load_from_file(file_path)
 
@@ -175,7 +176,7 @@ def fetch_from_catalog(self, resources, catalog):
         'ontograph.loader.PoochDownloaderAdapter',
         lambda cache_dir: DummyDownloader(),
     )
-    with pytest.raises(NotImplementedError):
+    with pytest.raises(RuntimeError):
         pronto_loader._download_ontology('ado', 'obo')
 
 

tests/test_queries/test_introspection.py

@@ -81,15 +81,15 @@ def test_get_path_between_ancestor_descendant(dummy_introspection):
     # C is a direct child of Z (root), path should be Z -> C
     path = dummy_introspection.get_path_between('Z', 'C')
     ids = [node['id'] for node in path]
-    print(ids)
+
     assert ids == ['Z', 'C']
 
 
 def test_get_path_between_descendant_ancestor(dummy_introspection):
     # C is a direct child of Z (root), path should be Z -> C
     path = dummy_introspection.get_path_between('C', 'Z')
     ids = [node['id'] for node in path]
-    print(ids)
+
     assert ids == ['Z', 'C']
 
 
@@ -124,7 +124,7 @@ def test_get_trajectories_from_root_single_path(dummy_introspection):
     assert isinstance(trajectories, list)
     assert len(trajectories) == 1
     ids = [node['id'] for node in trajectories[0]]
-    print(ids)
+
     # Should start at F and end at root Z
     assert ids[0] == 'Z'
     assert ids[-1] == 'F'
@@ -135,7 +135,7 @@ def test_get_trajectories_from_root_multiple_paths(dummy_introspection):
     # If a term has multiple paths to root (multiple inheritance), all should be returned
     # For dummy ontology, let's assume D has two parents: A and B, both under Z
     trajectories = dummy_introspection.get_trajectories_from_root('Y')
-    print(trajectories)
+
     assert isinstance(trajectories, list)
     # Should have at least one trajectory
     assert len(trajectories) >= 1

tests/test_queries/test_relations.py

@@ -75,8 +75,10 @@ def test_is_ancestor_invalid_id(dummy_relations):
 def raise_runtime_error(*a, **kw):
     raise RuntimeError('fail')
 
+
 def test_is_ancestor_exception(dummy_relations, monkeypatch):
     # Patch get_ancestors to raise Exception
+    # Accessing _OntologyRelations__navigator directly is intentional for testing.
     monkeypatch.setattr(
         dummy_relations._OntologyRelations__navigator,
         'get_ancestors',
@@ -108,6 +110,7 @@ def test_is_descendant_self(dummy_relations):
 
 
 def test_is_descendant_exception(dummy_relations, monkeypatch):
+    # Accessing _OntologyRelations__navigator directly is intentional for testing.
     monkeypatch.setattr(
         dummy_relations._OntologyRelations__navigator,
         'get_descendants',