Closed noornoorie closed 2 months ago
First of all, good job on that one! I have some comments:
I think the processor filters should also filter whole workflows because we want to see workflows that contain a certain processor and hide workflows that do not contain the selected processor(s). Also it should be a multiselect filter switching that filter to inspect 2 or more processors may be tidious.
I've changed processor dropdown to multiselect as you requested. However, I'm not sure about the other requirement. Should the two dropdowns be mutually connected so that changing one would change the values of the other? Or should it work the opposite of what's currently implemented - selecting processor should change the values of workflow dropdown?
... However, I'm not sure about the other requirement. Should the two dropdowns be mutually connected so that changing one would change the values of the other? Or should it work the opposite of what's currently implemented - selecting processor should change the values of workflow dropdown?
I think there is a misunderstanding what the filter should do. I think it should be like "show only GT items that contain selected processors". Not just show the selected processors and hide not selected ones, this gives no benefit, it just hides some array items.
I see now also the "workflow filter" should behave the same.
So each of the filters above the GT items list should filter out GT items to enable users to find better their desired GT item. So, no, there is no connection between the filters themselves (yet). It is basically an AND conjuction of selected filter values.
I believe I'm already filtering the GT items here?
const gtList = computed<GroundTruth[]>(() => {
return workflowsStore.gt.filter(({ id, metadata }) => {
let flag = filtersStore.gt.findIndex(({ value }) => value === id) > -1
if (flag && selectedDateRange.value) {
const splitDateRange = selectedDateRange.value.value.split('-')
const from = splitDateRange[0]
const to = splitDateRange[1]
flag = flag && (metadata.time.notBefore === from && metadata.time.notAfter === to)
}
if (flag && selectedWorkflow.value) {
flag = flag && workflowsStore.getRuns(id, selectedWorkflow.value.value).length > 0
}
if (flag && selectedWorkflowSteps.value.length > 0) {
selectedWorkflowSteps.value.forEach(({ value }) => {
if (!flag) {
return
}
flag = flag && workflowsStore.getRuns(id).findIndex(({ metadata }) => {
return metadata.workflow_steps.findIndex(({ id }) => id === value) > -1
}) > -1
})
}
return flag
})
})
This is what happens when we select something from the processor filter. It seems to filter out the display in the workflows table. I realize that the issue is that we might never even able to filter any GT items because of course we have each processor in all 4 workflows in all GT items. So this filter might be quite useless at this point and only start working when we apply some different workflows on different GT items. Maybe @MareenGeestmann can say somethign about this.
As for now all GT documents were run with all displayed workflows filtering by processors will not reduce the list of documents. Therefore, we do not need to implement it (for now).
@MareenGeestmann So should I just remove the GT list filtering part - keeping the dropdowns to control the visibility of workflows and processors inside the GT items, or should I drop the pull request altogether?
@MareenGeestmann So should I just remove the GT list filtering part - keeping the dropdowns to control the visibility of workflows and processors inside the GT items, or should I drop the pull request altogether?
Let us discuss this with @paulpestov.
I think we should keep the processor filter. We know that right now every GT item is executed with every workflow (and therefore every processor is present) but that might change in the future. I think the combination which GT is executed with which workflow is just a configuration thing of the Quiver backend and not a feature per se. So it CAN be changed very easily. For that case we will already have a good filter.
@MareenGeestmann So should I just remove the GT list filtering part - keeping the dropdowns to control the visibility of workflows and processors inside the GT items, ...
No, it should be the exact opposite: remove the control of visibility of workflows and processors inside the GT items and add the GT list filtering part.
As mentioned before, all filters should be about filtering GT items. Nothing should change within a GT item. Though, if we wish so, we should open a new issue for filtering information inside a GT item.
One more thing: could you set all checkboxes to checked at the processor filter? Right now they are initially all unchecked which is kinda false since we see "all GT items with all processors" at the first load.
@paulpestov i've coded such that the behavior is the same with all the processors checked and with all unchecked. Pre-selecting all the processors makes filtering them harder, as noted in issue #64. Do you have any advice regarding this issue?
I still think what I wrote in the comments above should be the correct behaviour. The search is just a little add-on and could be omitted.
@paulpestov I've added the checkboxes on page load as requested
Thanks, but what about the filtering GT stuff? That:
I think we should keep the processor filter. We know that right now every GT item is executed with every workflow (and therefore every processor is present) but that might change in the future. I think the combination which GT is executed with which workflow is just a configuration thing of the Quiver backend and not a feature per se. So it CAN be changed very easily. For that case we will already have a good filter.
@MareenGeestmann So should I just remove the GT list filtering part - keeping the dropdowns to control the visibility of workflows and processors inside the GT items, ...
No, it should be the exact opposite: remove the control of visibility of workflows and processors inside the GT items and add the GT list filtering part.
As mentioned before, all filters should be about filtering GT items. Nothing should change within a GT item. Though, if we wish so, we should open a new issue for filtering information inside a GT item.
Open: Hide row where workflow contains not the filtered processor. (keep all processors visible) Hide whole GT item when processor does not exist in all workflows.
Closes #45