Add filtering functionality to bids2table #6
Conversation
There was a problem hiding this comment.
Thanks for the PR @ReinderVosDeWael! Left a couple comments. And then I'll be glad to merge.
| try: | ||
| df = df[df["entities"][key].isin(value)] | ||
| except KeyError as exc_info: | ||
| raise exceptions.InvalidFilterError( |
There was a problem hiding this comment.
I don't think I would catch and re-raise a custom exception. I would just let the KeyError be raised. Just as informative I think and less code.
There was a problem hiding this comment.
For a developer like you or myself you're right. However, my idea here is to raise an error that is informative to someone who has never opened the source code. Consider someone who calls bids2table(..., filters={...}) If they get thrown a KeyError at line 140 at df = df[df["entities"][key].isin(value)] then they'd have to go through the stack trace and figure out from there that this key variable refers to one of their filters. That's a commitment beyond many end-users. If we're lucky they launch an issue for it, if we're unlucky they'll move to a different package. If they get thrown a InvalidFilterError then they would barely have to glance at the error description to know that it's a user error.
There was a problem hiding this comment.
Fair point. The default KeyError message is very minimal and would probably be confusing. How about we re-raise KeyError with a more informative message? In general, I prefer not to use custom exceptions unless there's no good fit among the built-in exceptions or I need to handle the exception specially.
| ] | ||
| } | ||
| ], | ||
| "outputs": [], |
There was a problem hiding this comment.
The table generation progress log is now missing. Not sure why. Perhaps a restart and run all would fix it?
|
Superseded by #17. |
This PR adds filtering functionality to bids2table. It can be invoked with lists of values (that are considered an OR), or single values (equivalent to a list of a single value). See example usage below.
Specifically, this PR adds the following:
_filtermethod to_bids2table.py_filter, including a workflow to run themexceptions.pyfor custom exceptions.