248 inconsistency between edgepopulationiter connections and edgesite…

…r connections (#249)

* resolve inconsistency between edge_population iter connections and edges iter connections

CHANGELOG.rst

@@ -1,21 +1,18 @@
 Changelog
 =========
 
-Version v2.1.0
+Version v3.0.0
 --------------
 
 New Features
 ~~~~~~~~~~~~
 - Added simulation config validation
-- Added a new  commandline subcommand: ``validate-simulation``
-- Added an alias ``validate-circuit`` for the old ``validate`` subcommand
-
-  - deprecated ``validate``
-
+- Added a new  commandline subcommands: ``validate-simulation``, ``validate-circuit``
 
 Breaking Changes
 ~~~~~~~~~~~~~~~~
-- Deprecated the commandline subcommand ``validate`` in favor of new ``validate-circuit`` command
+- Edge populations' ``iter_connections`` returns ``CircuitNodeId`` instead of ``int``
+- Removed the commandline subcommand ``validate`` in favor of new ``validate-circuit`` command
 
 
 Version v2.0.2

bluepysnap/circuit_validation.py

@@ -2,6 +2,7 @@
 
 The idea here is to not depend on libsonata if possible, so we can use this in all situations
 """
+
 import logging
 from pathlib import Path
 

bluepysnap/cli.py

@@ -1,12 +1,11 @@
 """The project's command line launcher."""
+
 import functools
 import logging
-import warnings
 
 import click
 
 from bluepysnap import circuit_validation, simulation_validation
-from bluepysnap.utils import Deprecate
 
 CLICK_EXISTING_FILE = click.Path(exists=True, file_okay=True, dir_okay=False)
 
@@ -43,27 +42,6 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
-@cli.command()
-@circuit_validation_params
-def validate(config_file, skip_slow, only_errors):
-    """[DEPRECATED] Validate Sonata circuit based on config file.
-
-    Args:
-        config_file (str): path to Sonata circuit config file
-        skip_slow (bool): skip slow tests
-        only_errors (bool): only print fatal errors
-    """
-    with warnings.catch_warnings():
-        # Making sure the warning is shown
-        warnings.simplefilter("always", DeprecationWarning)
-        Deprecate.warn(
-            "Calling circuit validation with 'validate' is deprecated. "
-            "Please use 'validate-circuit' instead."
-        )
-
-    circuit_validation.validate(config_file, skip_slow, only_errors)
-
-
 @cli.command()
 @circuit_validation_params
 def validate_circuit(config_file, skip_slow, only_errors):

bluepysnap/edges/edge_population.py

@@ -26,7 +26,7 @@
 from more_itertools import first
 
 from bluepysnap import query, utils
-from bluepysnap.circuit_ids import CircuitEdgeIds
+from bluepysnap.circuit_ids import CircuitEdgeIds, CircuitNodeId
 from bluepysnap.circuit_ids_types import IDS_DTYPE, CircuitEdgeId
 from bluepysnap.exceptions import BluepySnapError
 from bluepysnap.sonata_constants import DYNAMICS_PREFIX, ConstContainer, Edge
@@ -518,6 +518,41 @@ def _optimal_direction():
                     secondary_node_ids_used.add(conn_node_id)
                     break
 
+    def _add_circuit_ids(self, its):
+        """Completes the CircuitNodeId."""
+
+        return (
+            (
+                CircuitNodeId(self.source.name, source_id),
+                CircuitNodeId(self.target.name, target_id),
+                count,
+            )
+            for source_id, target_id, count in its
+        )
+
+    def _add_edge_ids(self, its):
+        """Completes the CircuitNodeId and adds the CircuitEdgeIds."""
+
+        return (
+            (
+                CircuitNodeId(self.source.name, source_id),
+                CircuitNodeId(self.target.name, target_id),
+                CircuitEdgeIds.from_dict({self.name: self.pair_edges(source_id, target_id)}),
+            )
+            for source_id, target_id, _ in its
+        )
+
+    def _omit_edge_count(self, its):
+        """Completes the CircuitNodeId and removes the edge count."""
+
+        return (
+            (
+                CircuitNodeId(self.source.name, source_id),
+                CircuitNodeId(self.target.name, target_id),
+            )
+            for source_id, target_id, _ in its
+        )
+
     def iter_connections(
         self,
         source=None,
@@ -557,13 +592,11 @@ def iter_connections(
         it = self._iter_connections(source_node_ids, target_node_ids, unique_node_ids, shuffle)
 
         if return_edge_count:
-            return it
+            return self._add_circuit_ids(it)
         elif return_edge_ids:
-            add_edge_ids = lambda x: (x[0], x[1], self.pair_edges(x[0], x[1]))
-            return map(add_edge_ids, it)
+            return self._add_edge_ids(it)
         else:
-            omit_edge_count = lambda x: x[:2]
-            return map(omit_edge_count, it)
+            return self._omit_edge_count(it)
 
     @property
     def h5_filepath(self):

bluepysnap/edges/edges.py

@@ -21,7 +21,6 @@
 
 from bluepysnap._doctools import AbstractDocSubstitutionMeta
 from bluepysnap.circuit_ids import CircuitEdgeIds, CircuitNodeIds
-from bluepysnap.circuit_ids_types import CircuitNodeId
 from bluepysnap.edges.edge_population import EdgePopulation
 from bluepysnap.exceptions import BluepySnapError
 from bluepysnap.network import NetworkObject
@@ -221,39 +220,6 @@ def pair_edges(self, source_node_id, target_node_id, properties=None):
             source=source_node_id, target=target_node_id, properties=properties
         )
 
-    @staticmethod
-    def _add_circuit_ids(its, source, target):
-        """Generator comprehension adding the CircuitIds to the iterator.
-
-        Notes:
-            Using closures or lambda functions would result in override functions and so the
-            source and target would be the same for all the populations.
-        """
-        return (
-            (CircuitNodeId(source, source_id), CircuitNodeId(target, target_id), count)
-            for source_id, target_id, count in its
-        )
-
-    @staticmethod
-    def _add_edge_ids(its, source, target, pop_name):
-        """Generator comprehension adding the CircuitIds to the iterator."""
-        return (
-            (
-                CircuitNodeId(source, source_id),
-                CircuitNodeId(target, target_id),
-                CircuitEdgeIds.from_dict({pop_name: edge_id}),
-            )
-            for source_id, target_id, edge_id in its
-        )
-
-    @staticmethod
-    def _omit_edge_count(its, source, target):
-        """Generator comprehension adding the CircuitIds to the iterator."""
-        return (
-            (CircuitNodeId(source, source_id), CircuitNodeId(target, target_id))
-            for source_id, target_id in its
-        )
-
     def iter_connections(
         self, source=None, target=None, return_edge_ids=False, return_edge_count=False
     ):
@@ -276,21 +242,13 @@ def iter_connections(
             raise BluepySnapError(
                 "`return_edge_count` and `return_edge_ids` are mutually exclusive"
             )
-        for name, pop in self.items():
-            it = pop.iter_connections(
+        for pop in self.values():
+            yield from pop.iter_connections(
                 source=source,
                 target=target,
                 return_edge_ids=return_edge_ids,
                 return_edge_count=return_edge_count,
             )
-            source_pop = pop.source.name
-            target_pop = pop.target.name
-            if return_edge_count:
-                yield from self._add_circuit_ids(it, source_pop, target_pop)
-            elif return_edge_ids:
-                yield from self._add_edge_ids(it, source_pop, target_pop, name)
-            else:
-                yield from self._omit_edge_count(it, source_pop, target_pop)
 
     def __getstate__(self):
         """Make Edges pickle-able, without storing state of caches."""

bluepysnap/query.py

@@ -1,4 +1,5 @@
 """Module to process search queries of nodes/edges."""
+
 import operator
 from collections.abc import Mapping
 from copy import deepcopy

bluepysnap/schemas/__init__.py

@@ -1,4 +1,5 @@
 """Schemas and schema parser."""
+
 from bluepysnap.schemas.schemas import (
     validate_circuit_schema,
     validate_edges_schema,

bluepysnap/schemas/schemas.py

@@ -1,4 +1,5 @@
 """Functions for schema based validation of circuit files."""
+
 from pathlib import Path
 
 import h5py

bluepysnap/simulation_validation.py

@@ -1,4 +1,5 @@
 """Standalone module that validates Sonata simulation. See ``validate-simulation`` function."""
+
 import contextlib
 import io
 import os

tests/data/reporting/create_reports.py

@@ -1,4 +1,5 @@
 """Taken from the libsonata lib."""
+
 import h5py
 import numpy as np
 

tests/test_cli.py

@@ -14,40 +14,31 @@
 @patch("bluepysnap.schemas.validate_nodes_schema", Mock(return_value=[]))
 @patch("bluepysnap.schemas.validate_edges_schema", Mock(return_value=[]))
 @patch("bluepysnap.schemas.validate_circuit_schema", Mock(return_value=[]))
-def test_cli_correct():
+def test_cli_validate_circuit_correct():
     runner = CliRunner()
-
-    with pytest.warns(
-        BluepySnapDeprecationWarning,
-        match="Calling circuit validation with 'validate' is deprecated",
-    ):
-        result = runner.invoke(cli, ["validate", str(TEST_DATA_DIR / "circuit_config.json")])
-
+    result = runner.invoke(cli, ["validate-circuit", str(TEST_DATA_DIR / "circuit_config.json")])
     assert result.exit_code == 0
     assert click.style("No Error: Success.", fg="green") in result.stdout
 
 
-def test_cli_no_config():
+def test_cli_validate_circuit_no_config():
     runner = CliRunner()
-    result = runner.invoke(cli, ["validate"])
+    result = runner.invoke(cli, ["validate-circuit"])
     assert result.exit_code == 2
     assert "Missing argument 'CONFIG_FILE'" in result.stdout
 
 
-@patch("bluepysnap.schemas.validate_nodes_schema", Mock(return_value=[]))
-@patch("bluepysnap.schemas.validate_edges_schema", Mock(return_value=[]))
-@patch("bluepysnap.schemas.validate_circuit_schema", Mock(return_value=[]))
-def test_cli_validate_circuit_correct():
-    runner = CliRunner()
-    result = runner.invoke(cli, ["validate-circuit", str(TEST_DATA_DIR / "circuit_config.json")])
-    assert result.exit_code == 0
-    assert click.style("No Error: Success.", fg="green") in result.stdout
-
-
 def test_cli_validate_simulation_correct():
     runner = CliRunner()
     result = runner.invoke(
         cli, ["validate-simulation", str(TEST_DATA_DIR / "simulation_config.json")]
     )
     assert result.exit_code == 0
     assert click.style("No Error: Success.", fg="green") in result.stdout
+
+
+def test_cli_validate_simulation_no_config():
+    runner = CliRunner()
+    result = runner.invoke(cli, ["validate-simulation"])
+    assert result.exit_code == 2
+    assert "Missing argument 'CONFIG_FILE'" in result.stdout