Add python 3.9 tests, ensure int64 in reports, fix warnings (#151)

* Add python 3.9 tests.
* Ensure that ids in frame reports are always np.int64 even when using libsonata 0.1.10 [NSETM-1766].
* Improve documentation
* Fix deprecation warnings.

Fixed pandas warnings:
- FutureWarning: The default dtype for empty Series will be 'object' instead of 'float64' in a future version. Specify a dtype explicitly to silence this warning.
- FutureWarning: Passing a set as an indexer is deprecated and will raise in a future version. Use a list instead.

.github/workflows/publish-sdist.yml

@@ -11,10 +11,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@master
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
       - name: Build a source tarball
         run:
             python setup.py sdist

.github/workflows/run-tox.yml

@@ -12,7 +12,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [3.7, 3.8]
+        python-version: [3.7, 3.8, 3.9]
 
     steps:
     - uses: actions/checkout@v2
@@ -28,10 +28,10 @@ jobs:
       run: |
         tox
     - name: Upload to codecov
-      if: ${{matrix.python-version == '3.8'}}
+      if: ${{matrix.python-version == '3.9'}}
       uses: codecov/codecov-action@v1
       with:
         fail_ci_if_error: false
         files: ./coverage.xml
         flags: pytest
-        name: "bluepysnap-py38"
+        name: "bluepysnap-py39"

CHANGELOG.rst

@@ -4,9 +4,18 @@ Changelog
 Version v0.13.1
 ---------------
 
+Improvements
+~~~~~~~~~~~~
+- Add python 3.9 tests.
+
+Bug Fixes
+~~~~~~~~~
+- Ensure that ids in frame reports are always np.int64 even when using libsonata 0.1.10.
+- Fix deprecation warnings.
+
 Removed
 ~~~~~~~
-- Drop python3.6 support
+- Drop python 3.6 support.
 
 
 Version v0.13.0

bluepysnap/_plotting.py

@@ -22,7 +22,7 @@
 
 from bluepysnap.exceptions import BluepySnapError
 from bluepysnap.sonata_constants import Node
-
+from bluepysnap.utils import IDS_DTYPE
 
 L = logging.getLogger(__name__)
 
@@ -133,8 +133,9 @@ def _update_raster_properties():
 
     report = filtered_report.report
 
-    dtype = spike_report[population_names[0]].nodes.property_dtypes[y_axis] if y_axis else None
-    if dtype and pd.api.types.is_categorical_dtype(dtype):
+    # use np.int64 if displaying node_ids
+    dtype = spike_report[population_names[0]].nodes.property_dtypes[y_axis] if y_axis else IDS_DTYPE
+    if pd.api.types.is_categorical_dtype(dtype):
         # this is to prevent the problems when concatenating categoricals with unknown categories
         dtype = str
     data = pd.Series(index=report.index, dtype=dtype)

bluepysnap/edges.py

@@ -102,15 +102,19 @@ def get(self, edge_ids=None, properties=None):   # pylint: disable=arguments-ren
         if edge_ids is None:
             raise BluepySnapError("You need to set edge_ids in get.")
         if properties is None:
-            Deprecate.warn("Returning ids with get/properties will be removed in 1.0.0."
-                           "Please use Edges.ids() instead.")
+            Deprecate.warn(
+                "Returning ids with get/properties is deprecated and will be removed in 1.0.0. "
+                "Please use Edges.ids instead."
+            )
             return edge_ids
         return super().get(edge_ids, properties)
 
     def properties(self, edge_ids, properties):
         """Doc is overridden below."""
-        Deprecate.warn("Edges.properties function will be deprecated in 1.0.0. Please use "
-                       "Edges.get instead.")
+        Deprecate.warn(
+            "Edges.properties function is deprecated and will be removed in 1.0.0. "
+            "Please use Edges.get instead."
+        )
         return self.get(edge_ids, properties)
 
     properties.__doc__ = get.__doc__
@@ -489,8 +493,10 @@ def _get(self, selection, properties=None):
         """Get an array of edge IDs or DataFrame with edge properties."""
         edge_ids = utils.ensure_ids(selection.flatten())
         if properties is None:
-            Deprecate.warn("Returning ids with get/properties will be removed in 1.0.0."
-                           "Please use EdgePopulation.ids() instead.")
+            Deprecate.warn(
+                "Returning ids with get/properties is deprecated and will be removed in 1.0.0. "
+                "Please use EdgePopulation.ids instead."
+            )
             return edge_ids
 
         if utils.is_iterable(properties):
@@ -614,8 +620,10 @@ def get(self, edge_ids, properties):
 
     def properties(self, edge_ids, properties):
         """Doc is overridden below."""
-        Deprecate.warn("EdgePopulation.properties function will be deprecated in 1.0.0. Please use "
-                       "EdgePopulation.get instead.")
+        Deprecate.warn(
+            "EdgePopulation.properties function is deprecated and will be removed in 1.0.0. "
+            "Please use EdgePopulation.get instead."
+        )
         return self.get(edge_ids, properties)
 
     properties.__doc__ = get.__doc__

bluepysnap/frame_report.py

@@ -106,9 +106,14 @@ def get(self, group=None, t_start=None, t_stop=None):
         if len(view.ids) == 0:
             return pd.DataFrame()
 
-        res = pd.DataFrame(data=view.data,
-                           columns=pd.MultiIndex.from_arrays(np.asarray(view.ids).T),
-                           index=view.times).sort_index(axis=1)
+        # cell ids and section ids in the columns are enforced to be int64
+        # to avoid issues with numpy automatic conversions and to ensure that
+        # the results are the same regardless of the libsonata version [NSETM-1766]
+        res = pd.DataFrame(
+            data=view.data,
+            columns=pd.MultiIndex.from_arrays(ensure_ids(view.ids).T),
+            index=view.times,
+        ).sort_index(axis=1)
 
         # rename from multi index to index cannot be achieved easily through df.rename
         res.columns = self._wrap_columns(res.columns)

bluepysnap/nodes.py

@@ -545,18 +545,68 @@ def get(self, group=None, properties=None):
                 - ``mapping``: return the properties of nodes matching a properties filter.
                 - ``None``: return the properties of all nodes.
 
-            properties (list): If specified, return only the properties in the list.
+            properties (list|str|None): If specified, return only the properties in the list.
                 Otherwise return all properties.
 
         Returns:
             value/pandas.Series/pandas.DataFrame:
-                If single node ID is passed as ``group`` and single property as properties returns
-                a single value. If single node ID is passed as ``group`` and list as property
-                returns a pandas Series. Otherwise return a pandas DataFrame indexed by node IDs.
+                The type of the returned object depends on the type of the input parameters,
+                see the Examples for an explanation of the different cases.
 
         Notes:
             The NodePopulation.property_names function will give you all the usable properties
             for the `properties` argument.
+
+        Examples:
+            Considering a node population composed by 3 nodes (0, 1, 2) and 12 properties,
+            the following examples show the types of the returned objects.
+
+            - If ``group`` is a single node ID and ``properties`` a single property,
+              returns a single scalar value.
+
+                >>> result = my_node_population.get(group=0, properties=Cell.MTYPE)
+                >>> type(result)
+                str
+
+            - If ``group`` is a single node ID and ``properties`` a list or None,
+              returns a pandas Series indexed by the properties.
+
+                >>> result = my_node_population.get(group=0)
+                >>> type(result), result.shape
+                (pandas.core.series.Series, (12,))
+
+                >>> result = my_node_population.get(group=0, properties=[Cell.MTYPE])
+                >>> type(result), result.shape
+                (pandas.core.series.Series, (1,))
+
+            - If ``group`` is anything other than a single node ID, and ``properties`` is a single
+              property, returns a pandas Series indexed by node IDs.
+
+                >>> result = my_node_population.get(properties=Cell.MTYPE)
+                >>> type(result), result.shape
+                (pandas.core.series.Series, (3,))
+
+                >>> result = my_node_population.get(group=[0], properties=Cell.MTYPE)
+                >>> type(result), result.shape
+                (pandas.core.series.Series, (1,))
+
+            - In all the other cases, returns a pandas DataFrame indexed by node IDs.
+
+                >>> result = my_node_population.get()
+                >>> type(result), result.shape
+                (pandas.core.frame.DataFrame, (3, 12))
+
+                >>> result = my_node_population.get(group=[0])
+                >>> type(result), result.shape
+                (pandas.core.frame.DataFrame, (1, 12))
+
+                >>> result = my_node_population.get(properties=[Cell.MTYPE])
+                >>> type(result), result.shape
+                (pandas.core.frame.DataFrame, (3, 1))
+
+                >>> result = my_node_population.get(group=[0], properties=[Cell.MTYPE])
+                >>> type(result), result.shape
+                (pandas.core.frame.DataFrame, (1, 1))
         """
         result = self._data
         if properties is not None:

bluepysnap/spike_report.py

@@ -106,7 +106,7 @@ def get(self, group=None, t_start=None, t_stop=None):
 
         if not res:
             return pd.Series(data=[], index=pd.Index([], name="times"),
-                             name=series_name, dtype=np.float64)
+                             name=series_name, dtype=IDS_DTYPE)
 
         res = pd.DataFrame(data=res, columns=[series_name, "times"]).set_index("times")[series_name]
         if self._sorted_by != "by_time":

setup.py

@@ -80,9 +80,9 @@ def __init__(self, *args, **kwargs):
     ],
     classifiers=[
         'Development Status :: 3 - Alpha',
-        'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: 3.7',
         'Programming Language :: Python :: 3.8',
+        'Programming Language :: Python :: 3.9',
         'Operating System :: POSIX',
         'Topic :: Scientific/Engineering',
         'Topic :: Utilities',

tests/test_edges.py

@@ -234,7 +234,10 @@ def test_get(self):
             self.test_obj.get(properties=["other2", "unknown"])
 
         ids = CircuitEdgeIds.from_dict({"default": [0, 1, 2, 3], "default2": [0, 1, 2, 3]})
-        tested = self.test_obj.get(ids, None)
+        with pytest.deprecated_call(
+            match="Returning ids with get/properties is deprecated and will be removed in 1.0.0"
+        ):
+            tested = self.test_obj.get(ids, None)
         assert tested == ids
 
         tested = self.test_obj.get(ids, properties=self.test_obj.property_names)
@@ -283,11 +286,14 @@ def test_get(self):
         with pytest.deprecated_call():
             self.test_obj.get(ids)
 
-    def test_properties(self):
+    def test_properties_deprecated(self):
         ids = CircuitEdgeIds.from_dict({"default": [0, 1, 2, 3], "default2": [0, 1, 2, 3]})
-        pdt.assert_frame_equal(self.test_obj.properties(ids, properties=["other2", "@source_node"]),
-                               self.test_obj.get(ids, properties=["other2", "@source_node"]),
-                               check_exact=False)
+        with pytest.deprecated_call(
+            match="Edges.properties function is deprecated and will be removed in 1.0.0"
+        ):
+            tested = self.test_obj.properties(ids, properties=["other2", "@source_node"])
+        expected = self.test_obj.get(ids, properties=["other2", "@source_node"])
+        pdt.assert_frame_equal(tested, expected, check_exact=False)
 
     def test_afferent_nodes(self):
         assert self.test_obj.afferent_nodes(0) == CircuitNodeIds.from_arrays(["default"], [2])
@@ -783,12 +789,24 @@ def test_get_4(self):
         with pytest.raises(BluepySnapError):
             self.test_obj.get([0], 'no-such-property')
 
-    def test_properties(self):
+    def test_get_without_properties_deprecated(self):
+        edge_ids = [0, 1]
+        with pytest.deprecated_call(
+            match="Returning ids with get/properties is deprecated and will be removed in 1.0.0"
+        ):
+            actual = self.test_obj.get(edge_ids, None)
+        expected = np.asarray(edge_ids, dtype=np.int64)
+        npt.assert_equal(actual, expected)
+
+    def test_properties_deprecated(self):
         ids = [0, 1, 2, 3]
         properties = ["@target_node", "@source_node"]
-        pdt.assert_frame_equal(self.test_obj.properties(ids, properties=properties),
-                               self.test_obj.get(ids, properties=properties),
-                               check_exact=False)
+        with pytest.deprecated_call(
+            match="EdgePopulation.properties function is deprecated and will be removed in 1.0.0"
+        ):
+            actual = self.test_obj.properties(ids, properties=properties)
+        expected = self.test_obj.get(ids, properties=properties)
+        pdt.assert_frame_equal(actual, expected, check_exact=False)
 
     def test_get_all_edge_ids_types(self):
         assert self.test_obj.get(0, Synapse.PRE_GID).tolist() == [2]

tests/test_frame_report.py

@@ -203,7 +203,7 @@ def test_get(self):
         pdt.assert_frame_equal(self.test_obj.get(CircuitNodeId("default", 2)), self.df.loc[:, [2]])
 
         # not from this population
-        pdt.assert_frame_equal(self.test_obj.get(CircuitNodeId("default2", 2)),  pd.DataFrame())
+        pdt.assert_frame_equal(self.test_obj.get(CircuitNodeId("default2", 2)), pd.DataFrame())
 
         pdt.assert_frame_equal(self.test_obj.get([2, 0]), self.df.loc[:, [0, 2]])
 

tests/test_nodes.py

@@ -656,7 +656,9 @@ def test_get(self):
         )
 
         assert _call("Node0_L6_Y", properties=[Cell.X, Cell.MTYPE, Cell.LAYER]).empty
-        assert _call(1, properties={Cell.MTYPE}).tolist() == ["L6_Y"]
+        assert _call(1, properties=[Cell.MTYPE]).tolist() == ["L6_Y"]
+        assert _call([1], properties=Cell.MTYPE).tolist() == ["L6_Y"]
+        assert _call([1, 2], properties=Cell.MTYPE).tolist() == ["L6_Y", "L6_Y"]
         with pytest.raises(BluepySnapError):
             _call(0, properties='no-such-property')
         with pytest.raises(BluepySnapError):

tests/test_spike_report.py

@@ -21,7 +21,7 @@ def _get_index(ids):
         return pd.Index(ids, name="times")
     if len(node_ids) == 0:
         # removing warning
-        return pd.Series(node_ids, index=_get_index(index), name=name, dtype=np.float64)
+        return pd.Series(node_ids, index=_get_index(index), name=name, dtype=np.int64)
     return pd.Series(node_ids, index=_get_index(index), name=name)
 
 

tests/test_utils.py

@@ -25,7 +25,6 @@ def test_is_iterable():
 
 def test_is_node_id():
     assert test_module.is_node_id(1)
-    assert test_module.is_node_id(np.int(1))
     assert test_module.is_node_id(np.int32(1))
     assert test_module.is_node_id(np.uint32(1))
     assert test_module.is_node_id(np.int64(1))

tox.ini

@@ -4,12 +4,12 @@ name = bluepysnap
 [tox]
 envlist =
     lint
-    py{37,38}
+    py{37,38,39}
 
 ignore_basepython_conflict = true
 
 [testenv]
-basepython=python3.8
+basepython=python3.9
 deps =
     mock
     pytest
@@ -57,4 +57,5 @@ convention = google
 [gh-actions]
 python =
   3.7: py37
-  3.8: py38, lint, docs
+  3.8: py38
+  3.9: py39, lint, docs