Closed CamphynR closed 3 weeks ago
Looks good to me. The only thing is that in principle the typing for this argument has to be adapted. Maybe @cozzyd knows that to write there. But I am also happy to ignore that.
You should be able to change the type to a Union of the existing type and a Sequence type, I think? Otherwise people won't know how to use this correctly by looking at the type signature.
While I think this is a fine way to do it, I wonder if a more flexible approach would be to provide a helper functor that looks something like:
class SelectorAll:
def __init__(self, selectors : Sequence[Callable[EventInfo]]):
self._selectors = selectors
def __call__(self, info : EventInfo):
return numpy.all([selector(info) for selector in _selectors])
Then you could also define SelectorAny, SelectorNoneOf, SelectorOnlyOneOf, SelectorAtLeast2, etc.
Then you would be able to use it as: iterate( ... , selector = SelectorAll((A,B,C)))
change the type to a Union of the existing type and a Sequence of the existing type
@CamphynR can you try this. I hope this does not become unreadable
@cozzyd interesting! I kind of like the idea but I wonder if we should keep it as simple as possible. I guess with your class one could still pass single functions/lambdas but no lists anymore.
I would like for Ruben to go a head with the PR so he can continue working. But we can think of improving this. Implementing selectors as a class could us also allow to implement some nice logging of the filtering.
We could also think about filtering base of waveforms ?
What I mean is the interface already supports passing a class (as long as the class is a functor and has the correct call method) without any changes, but that still remains true even with the changes and passing a sequence can certainly be a bit more ergonomic.
adding option to pass several selectors to dataset.iterate function