-
Notifications
You must be signed in to change notification settings - Fork 3
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
To discuss: Replace attribute filters rather than merging them #2155
Comments
Side comment: the choice of replacing (rather than merging) this kind of filters also makes it much easier to look at a job and immediately understand which filters where applied, because it does not require the user to be aware of the underlying merging rules (which are far from intuitive). |
I could see something like this. For the UI: As long as we pre-fill the dataset attribute filters, we can just take the final selection in the run modal as the needed filters without having to combine them again |
Job attribute filters always take priority over dataset attribute filters. The dataset attribute filters are replaced by the job filters. What if a task sets an output attribute filter? Do we even allow that? We probably should not Types: First set by datasets type filters. Overwritten by workflow task input type filter. Workflow task output can set new types that overwrite both before. |
Side discussion: This is about setting smart defaults, not stopping users from setting other filters. |
In general, merging different filters may be complex - especially so in the upcoming version where attributes are associated to a list of possible values for a given key (e.g. well equal to B03 or B04 or B05).
Example: what does it mean to "merge" filters well=[B03, B04, B05] and well=[B03, B06]? What is the expected outcome?
We can clearly find the most reasonable answer, and implement it.
This would be even more complex if we have both include and exclude (which we actually propose to not pursue - ref #2154).
Note that this definition is strictly necessary for the interactive definition of job filters, as in fractal-analytics-platform/fractal-web#682 (comment). In this modal we have to create the "patch" filters which would produce the expected list of images when combined with existing filters.
With @mfranzon, we suggest to take a more clear direction - and state that attribute filters are never merged, but only replaced by highest-priority values.
Example:
old_attribute_filter = {well=[B03, B04, B05], anotherkey: [value1, value2]}
new_attribute_filter = {well=[B03, B06]}
resulting_filter = {well=[B03, B06], anotherkey: [value1, value2]}
Note: none of this applies to type filters, because there is no ambiguity in merging key=True with a higher-priority key=False. It's already a replacement. This is connected to our proposal to also split attributes and types into different db/API field - ref #2153.
The text was updated successfully, but these errors were encountered: