Feature/new filter #1068

kloss-o · 2022-02-04T11:43:34Z

Until now it was not so easy to filter for multiple values with the filter function in neo. To solve this problem, I introduced "FilterConditions". These can be provided to the filter function as the value of a dictionary.

An example of the syntax:

seg.filter({“electrode_id”: FilterCondition) or seg.filter(electrode_id = FilterCondition)

In order to be able to filter for several electrode_ids, there are some objects that inherit from FilterCondition and provide different filtering options.

For example, seg.filter({"electrode_id": neo.FilterIsIn([0, 1, 5]) can be used to filter for all objects with electrode_id 0, 1 or 5.

List of all currently added FilterConditions:

-FilterEqual(x), returns id == x

-FilterIsNot(x), returns id != x

-FilterLessThan(x), returns id < x

-FilterLessThanEqual(x), returns id <= x

-FilterGreaterThan(x), returns id > x

-FilterGreaterThanEqual(x), returns id >= x

-FilterIsIn([x, y, z]), returns id == x or id == y or id == z

-FilterInRange(a, b), returns a <= id <= b

Additional FilterConditions can be added at any time and without much effort.

pep8speaks · 2022-02-04T11:43:37Z

Hello @kloss-o! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-06-28 12:08:16 UTC

JuliaSprenger

Thanks for looking into improving the filter functionality! This has been on our todo list for quite some time. Your approach is adding a number of new classes to neo. Intuitively this looks like an approach that might add a bit of overhead to filtering. Did you also run some performance tests with your new implementation? @samuelgarcia What do you think?

neo/core/container.py

neo/test/coretest/test_container.py

Co-authored-by: Julia Sprenger <julia.sprenger@rwth-aachen.de>

kloss-o · 2022-02-10T13:30:44Z

Thank you for your feedback and suggestions. I'm looking into it.

mdenker · 2022-02-10T13:59:13Z

Thanks for the PR, Oliver, and for reviewing the code, Julia.

Regarding performance, there is actually another follow-up PR by @kloss-o in prep that would improve filtering speed quite a bit, but unrelated to this concrete implementation. @kloss-o , I think when you did some benchmarking the FilterCondition implementation (without further optimization) was comparable in speed to the traditional implementation?

The idea behind the different Filter classes was, for one, that it would allow users to also define their own "custom" filters as a filter function, like (in pseudocode):

class FilterGoodElectrodesSurroundedByGoodOnes(FilterCondition)
    def test(electrode):
       return is_good_quality[electrode] and np.all(is_good_quality[i] for i in neighbor_electrodes(electrode])

Secondly, that it would not require any new or ambiguous syntax (e.g., something like ["property", "<=", 5]) in the function call to filter(). In particular, the neo filter function would continue to work as before with the existing syntax, but adds additional functionality.

However, I think there are many other options. An alternative could be to revert to, e.g., simple filter functions, at the expense of being slightly more ambiguous.

Co-authored-by: Julia Sprenger <julia.sprenger@rwth-aachen.de>

Moritz-Alexander-Kern · 2022-05-09T10:05:31Z

The following notebook is intended to show the application of the 'new filter' function in practice.
The use cases were derived from the Neo - Elephant tutorials and aim to demonstrate how the suggested 'new filter' improves the workflow.

Link to notebook tutorial_new_filter_examples.ipynb:

GitHub gist: https://gist.github.com/Moritz-Alexander-Kern/afdfb707bde9052c6cd02ccddd2c4b9e
GIN: https://gin.g-node.org/NeuralEnsemble/neo_elephant_teaching/src/new_filter/tutorial_new_filter_examples.ipynb

JuliaSprenger · 2022-05-11T19:02:01Z

Here some notes on the discussion of this PR during the neo core meeting today.

JuliaSprenger · 2022-05-31T09:29:28Z

Hi @kloss-o @Moritz-Alexander-Kern any news on this? Tell me if the notes from the last meeting are not detailed enough for you to continue on this topic.

Review new filter

# Conflicts: # neo/core/container.py # neo/test/coretest/test_container.py

apdavison

I think this is in good shape now and almost ready to merge. I have suggested some changes to filter class names, and I would also prefer it if all whitespace-only changes were reverted, to make it easier to see where the code has actually changed. The most important change, I think, is to keep the functions in their own module and do not import them into the top-level namespace, i.e. they should be used as

from neo.filters import LessThan

or

import neo.filters as nf

...nf.LessThan...

apdavison · 2023-07-28T09:06:03Z

neo/core/__init__.py

@@ -35,6 +37,11 @@
 from neo.core.analogsignal import AnalogSignal
 from neo.core.irregularlysampledsignal import IrregularlySampledSignal

+# Import FilterClasses
+from neo.core.filters import Equal, IsIn, IsNot,\


I don't think the filters should be imported into the top-level namespace,
rather just from neo.core import filters

Agree, I used from neo.core import filters

apdavison · 2023-07-28T09:07:39Z

neo/core/container.py

@@ -18,27 +20,28 @@ def unique_objs(objs):
    using the "is" test.
    """
    seen = set()
-    return [obj for obj in objs
-            if id(obj) not in seen and not seen.add(id(obj))]
+    return [obj for obj in objs if id(obj) not in seen and not seen.add(id(obj))]


please revert all white-space only changes, they make it harder to see the substantial changes.

White-space only changes removed.

apdavison · 2023-07-28T09:08:47Z

neo/core/container.py



 def filterdata(data, targdict=None, objects=None, **kwargs):
    """
-    Return a list of the objects in data matching *any* of the search terms
+    Return a list of child objects matching *any* of the search terms


child of what? I think the original docstring is more understandable, perhaps put "objects in data" to make it clear that we're referring to the variable named data.

Reverted to:
Return a list of the objects in data matching *any* of the search terms

apdavison · 2023-07-28T09:11:45Z

neo/core/container.py

                    results.append(obj)

+    # remove duplicates from results
+    res = []
+    [res.append(result) for result in results if not any([result is elem for elem in res])]


This looks as though it will be slow - perhaps use a set to eliminate duplicates?

I tried with:

res = list(set(results))

but I get:

> res = list(set(results)) E TypeError: unhashable type: 'SpikeTrain'

any ideas/help?

perhaps use the Python id() function?, e.g.,

unique_results = { id(res): res for res in results }.values()

Perfect, thanks!

Used:

# remove duplicates from results results = list({ id(res): res for res in results }.values())

apdavison · 2023-07-28T09:12:35Z

neo/core/container.py

@@ -188,13 +195,12 @@ class Container(BaseNeo):
    # Containers that are listed when pretty-printing
    _repr_pretty_containers = ()

-    def __init__(self, name=None, description=None, file_origin=None,
-                 **annotations):
+    def __init__(self, name=None, description=None, file_origin=None, **annotations):


same comment as above about whitespace-only changes

Removed whitespace-only changes.

apdavison · 2023-07-28T09:15:33Z

neo/core/filters.py

+        raise NotImplementedError()
+
+
+class Equal(FilterCondition):


I would call this Equals or EqualTo

Used Equals

apdavison · 2023-07-28T09:15:54Z

neo/core/filters.py

+        return x != self.control
+
+
+class LessThanEqual(FilterCondition):


LessThanOrEquals or LessThanOrEqualTo

Used LessThanOrEquals

apdavison · 2023-07-28T09:17:40Z

neo/core/filters.py

+        return x <= self.control
+
+
+class GreaterThanEqual(FilterCondition):


GreaterThanOrEquals or GreaterThanOrEqualTo

Used GreaterThanOrEquals

…, as no duplicate keys can exist for a dict, duplicates will be removed with this

…lass

Moritz-Alexander-Kern · 2023-08-02T12:22:47Z

Hey @apdavison and @JuliaSprenger ,
Thank you for the reviews and the valuable feedback you provided.
I considered all your comments and made changes to the code.

If you have any further suggestions or if there are any remaining issues, please don't hesitate to let me or @mdenker know.

kloss-o and others added 10 commits November 10, 2021 10:10

created new classes for new filter-method

7ebad1a

created new classes for new filter-method

2ff6774

added new filters. Ready for testing

cc9f648

finished a few simple tests for new filter-objects

ac68e8d

added more tests

7b9210c

old filter-functionaltiy added again + tests

e464fbc

added doc

1d9bd8b

added doctrings to sphinx documentation

5587582

renamed all FilterConditions to match python standards

03c4253

pep8 standards

06d9b55

Oliver Kloß added 2 commits February 8, 2022 13:07

fixed remaining pep8 issues

fd34add

shortening lines

3692618

mdenker linked an issue Feb 8, 2022 that may be closed by this pull request

Advanced filter functionality #391

Closed

mdenker added discussion enhancement labels Feb 8, 2022

JuliaSprenger reviewed Feb 10, 2022

View reviewed changes

kloss-o and others added 3 commits February 10, 2022 14:13

Update neo/core/container.py

2d1af00

Co-authored-by: Julia Sprenger <julia.sprenger@rwth-aachen.de>

Update neo/core/container.py

62558c8

Co-authored-by: Julia Sprenger <julia.sprenger@rwth-aachen.de>

Update neo/core/container.py

8b7bd40

Co-authored-by: Julia Sprenger <julia.sprenger@rwth-aachen.de>

kloss-o and others added 3 commits February 17, 2022 10:15

Update neo/test/coretest/test_container.py

528514c

Co-authored-by: Julia Sprenger <julia.sprenger@rwth-aachen.de>

changed a few things based on feedback on pull request

4e98709

fixed last pep8speaks issue

75acb73

JuliaSprenger added this to the 0.11.0 milestone Mar 3, 2022

moved filters to separate module and renamed them

417fcad

Moritz-Alexander-Kern and others added 2 commits July 21, 2023 14:39

Merge pull request #42 from INM-6/review_new_filter

b3800ae

Review new filter

Merge branch 'master' into feature/newFilter

17666ac

# Conflicts: # neo/core/container.py # neo/test/coretest/test_container.py

apdavison requested changes Jul 28, 2023

View reviewed changes

Moritz-Alexander-Kern and others added 24 commits July 28, 2023 15:20

do not import filters individually into top level namespace

bf1c51d

revert to previous version of docstring of docstring for filterdata

4e1df3b

remove whitespace changes

f9e1d29

changed class names for GreaterThanEqual, LessThanEqual and Equal

21f829a

adapt unittests according to new class names

e46e9ed

eliminate duplicates with a dict comprehension, using id() as the key…

f183572

…, as no duplicate keys can exist for a dict, duplicates will be removed with this

remove else

f50f222

replace list with generator before returning tuple

dd9d2f0

use isinstance to check for type

56c2caf

change names, add docstrings to classes

f8a56ea

use abstract base class from the abc module for the FilterCondition c…

54373b9

…lass

fix typo in docstring

4616987

Merge branch 'NeuralEnsemble:master' into feature/newFilter

812ed39

undo returns for tuples

afdc1f5

add module docstring for filters.py

d124f1b

fix doctrings and class names

d50cb3b

refactor unit tests

ca82dcd

add tuples and sets to IsIn class

70a425e

simplify evaluate in IsIn

dd5ec83

further simplification of unit tests

2407353

add typehints

fd32d6a

add docstring to unit test

d655ac6

refactor naming for x to compare and z to control

9c30bde

fix typo and refactor filterdata

dedfc1b

apdavison approved these changes Jan 29, 2024

View reviewed changes

apdavison merged commit 3af1010 into NeuralEnsemble:master Jan 29, 2024
41 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/new filter #1068

Feature/new filter #1068

kloss-o commented Feb 4, 2022

pep8speaks commented Feb 4, 2022 •

edited

Loading

JuliaSprenger left a comment

kloss-o commented Feb 10, 2022

mdenker commented Feb 10, 2022

Moritz-Alexander-Kern commented May 9, 2022

JuliaSprenger commented May 11, 2022

JuliaSprenger commented May 31, 2022

apdavison left a comment

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

apdavison Jul 28, 2023

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern commented Aug 2, 2023 •

edited

Loading

		return x != self.control


		class LessThanEqual(FilterCondition):

		return x <= self.control


		class GreaterThanEqual(FilterCondition):

Feature/new filter #1068

Feature/new filter #1068

Conversation

kloss-o commented Feb 4, 2022

pep8speaks commented Feb 4, 2022 • edited Loading

Comment last updated at 2022-06-28 12:08:16 UTC

JuliaSprenger left a comment

Choose a reason for hiding this comment

kloss-o commented Feb 10, 2022

mdenker commented Feb 10, 2022

Moritz-Alexander-Kern commented May 9, 2022

JuliaSprenger commented May 11, 2022

JuliaSprenger commented May 31, 2022

apdavison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Moritz-Alexander-Kern Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Moritz-Alexander-Kern commented Aug 2, 2023 • edited Loading

pep8speaks commented Feb 4, 2022 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern Jul 28, 2023 •

edited

Loading

Moritz-Alexander-Kern commented Aug 2, 2023 •

edited

Loading