Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul global filtering of participants #344

Merged
merged 1 commit into from
Dec 16, 2023

Conversation

pvandyken
Copy link
Contributor

@pvandyken pvandyken commented Oct 18, 2023

--participant-filter and --exclude-participant filter have had a couple of bugs over the past few versions:

  1. Both terms would apply to every single component, even components without the subject entity. These components would have all of their entries filtered out
  2. Using --exclude-participant-filter would turn on regex matching mode for every single filter. This changed the meaning of the filters (e.g. allowing partial matches), leading to workflow disruption

The overhaul fixes both bugs, while improving the organization of the
input generation code.

Additionally, the documentation states the magic filter regex_search: True can be used to enable regex searching for a block of filters. This hasn't worked for the past few versions. Rather than fix it, the behaviour has been silently disabled in preparation for an overhaul of the regex filtering api

Resolves #303
Resolves #216

@pvandyken pvandyken added the bug Something isn't working label Oct 18, 2023
@pvandyken pvandyken force-pushed the fix/participant-label-filtering branch from b7e52cb to 95240f9 Compare December 11, 2023 21:30
@pvandyken pvandyken marked this pull request as ready for review December 11, 2023 22:49
Copy link
Contributor

@kaitj kaitj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This largely looked good to me - I just wanted to take another look through it before approving it to make sure I didn't miss anything. Thanks for tackling this!

@pvandyken
Copy link
Contributor Author

Just want a second review on this one. @Bradley-Karat, @myousif9, are one of you able to take a look?

@Bradley-Karat
Copy link

Everything looks good to me!

@github-actions github-actions bot requested review from akhanf and kaitj December 14, 2023 16:40
@pvandyken pvandyken force-pushed the fix/participant-label-filtering branch 4 times, most recently from ee116bb to 2559b09 Compare December 16, 2023 16:32
--participant-filter and --exclude-participant filter have had a couple
of bugs over the past few versions:

1. Both terms would apply to every single component, even components
   without the subject entity. These components would have all of their
   entries filtered out
2. Using --exclude-participant-filter would turn on regex matching
   mode for every single filter. This changed the meaning of the filters
   (e.g. allowing partial matches), leading to workflow disruption

The overhaul fixes both bugs, while improving the organization of the
input generation code.

Additionally, the documentation states the magic filter `regex_search:
True` can be used to enable regex searching for a block of filters. This
hasn't worked for the past few versions. Rather than fix it, the
behaviour has been silently disabled in preparation for an overhaul of
the regex filtering api. Mention of this feature has been removed from
documentation

Resolves khanlab#303
Resolves khanlab#216
@pvandyken pvandyken force-pushed the fix/participant-label-filtering branch from 2559b09 to 0adbef1 Compare December 16, 2023 18:47
@pvandyken pvandyken merged commit 80ed72d into khanlab:main Dec 16, 2023
28 checks passed
@pvandyken pvandyken deleted the fix/participant-label-filtering branch December 16, 2023 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants