Skip to content
This repository was archived by the owner on May 8, 2025. It is now read-only.

Adds feature for filtering annotations by shapes and user in Largo #193

Merged
merged 166 commits into from
Apr 25, 2025

Conversation

dbrembilla
Copy link
Contributor

This PR (partially) addresses issue biigle/core#1166 by introducing a filtering tab in Largo for users and shapes.
I also introduced tests for the new filtering functionalities on the backend side.

What I hoped to achieve, was a system that could be expanded upon. Please let me know if this is satisfactory.
Something that I am not 100% convinced is the fact that the filters get loaded on click - I was not able to pass the information from the API in any other way. Is it okay to you?

To test this, try to use filters to change the displayed largo annotations and check that no unexpected behaviour is present (e.g. annotations that should not be there that appear, other largo features stop working etc.)

…e, and integrate with the component.

The component is still not working as expected, however API routes are working.
This feature should be improved by making it load shapes from the DB on load rather than on click. Temporary checkpoint because of change of pc.
This commit implements the filtering functionality for the tab, which now fetches only the images required. For now the only filter implemented is the `shape` filter, but this implementation should allow for an easy expansion with any other filter.
This reverts commit fd29dd1.
This commit inadvertently introduces an autoformatting that makes the diffs unreadable.
Readds filtering functionality for shapes, that allows for expanding with other filters.
This commit implements the possibility of filtering images by users. Implements the select button as well as the js components required.
…unctions

In this commit, code that was previously imcomplete (e.g. API calls, filtering tab loading user names incorrectly)
gets fixed by refining functions and missing API calls.
@dbrembilla dbrembilla requested a review from mzur October 31, 2024 11:00
@dbrembilla dbrembilla self-assigned this Oct 31, 2024
@dlangenk
Copy link
Member

Wow. Cool PR. Thanks Davide

Copy link
Member

@mzur mzur left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! High-level comments first (in no particular order). I'll look at the code in a later review.

  • The BIIGLE modules track their compiled assets (JS/CSS) as well. So once you have something ready to review/publish, please run npm run prod once and commit the minified assets. Otherwise you end up with a 4k lines JS file like now.

  • When a filter is active, the sidebar tab must be highlighted. Compare this with the sorting tab when a sorting is active:

    image

    This is important so users don't forget about the filters once the tab is closed.

  • There is no need to make an API call to fetch the available annotation shapes. You can hard-code them in the template. Either directly as HTML or you populate a JS variable (with biigle.$declare() and biigle.$require()). Also, if Largo is opened for a single volume and it is not a video volume, the WholeFrame shape should not be shown.

  • To make the objective speed faster, you can perform the API call to fetch available users when the filter tab is open (instead of when the select input is focussed). Then the response will hopefully be already there when the user selects the input.

  • Ideally, I want the filter tab here to work similar to the filter tab of the volume overview (except the flag/filter switch and the show filenames button):

    image

    If you want you can implement it this way here. Otherwise we can keep your UI with the select inputs as first version and then update the UI later.

There were two things that didn't work for me:

  • When I open Largo for a whole project, I get an empty response from the "users-with-annotations" API endpoint.

  • When I click on a label (with annotations) and thenn immediately click on another label (without annotations), it shows the message "no annotations" for both labels. I even see a flash of annotation patches of the first label while it loads. But somehow the caching is messed up afterwards. I don't see this behavior in the version on biigle.de so I assume that it has something to do with your changes. Here is an example:

    Screencast.from.04.11.2024.14.42.57.webm

This commit  introduces highlighting to the filters tab by using  the activeFilters property
…alues of these filters

This commit changes fundamentally the way the filters are loaded; in particular, they are loaded not  on click, rather on page load.
To do so, the filters are loaded and then injected inside the template view, removing the need of further API calls.
This commit introduces a system that allows the selection of multiple values in the filter tab, so that it is now possible to combine multiple filters.
This comes with logical operators (is, is not, or, and). Changes the functioning of the API endpoints for filters of users and shapes as well.
In a further commit, tests will be introduced.
@dbrembilla
Copy link
Contributor Author

dbrembilla commented Nov 7, 2024

* The BIIGLE modules track their compiled assets (JS/CSS) as well. So once you have something ready to review/publish, please run `npm run prod` once and commit the minified assets. Otherwise you end up with a 4k lines JS file like now.

I am not able to run npm run prod for some reason. Can you test whether you can do it and push it? It might be a problem with my setup, IDK. EDIT: I managed to fix it, npm run prod fails silently with node 22. I created an issue about that biigle/core#984

* When a filter is active, the sidebar tab must be highlighted. Compare this with the sorting tab when a sorting is active:

This should be done now!

* There is no need to make an API call to fetch the available annotation shapes. You can hard-code them in the template. Either directly as HTML or you populate a JS variable (with `biigle.$declare()` and `biigle.$require()`). Also, if Largo is opened for a single volume and it is not a video volume, the WholeFrame shape should not be shown.

Done for the most part, need to remove the WholeFrame shape. EDIT: I fixed this part

* To make the objective speed faster, you can perform the API call to fetch available users when the filter tab is open (instead of when the select input is focussed). Then the response will hopefully be already there when the user selects the input.

I managed to add this as the page loads, lmk if it seems good to you!

* Ideally, I want the filter tab here to work similar to the filter tab of the volume overview (except the flag/filter switch and the show filenames button):

This took almost the whole week, but it seems to work now. Not the prettiest, but I'd like to now if the way it functions now is good for you. It was fun, and I hope the code is not too messy.

* When I open Largo for a whole project, I get an empty response from the "users-with-annotations" API endpoint.

Since there is no API anymore, it should not appear now!

* When I click on a label (with annotations) and thenn immediately click on another label (without annotations), it shows the message "no annotations" _for both labels_. I even see a flash of annotation patches of the first label while it loads. But somehow the caching is messed up afterwards. I don't see this behavior in the version on biigle.de so I assume that it has something to do with your changes. Here is an example:

I was not able to reproduce this, but then again with all the new changes a lot has been modified and it could be that I inadvertently fixed it while changing something else.

Thanks for the feedback!

… with filters

This commit makes the loading of filtered annotations into a different promise.
A bug of pinned images showing up during filtering is also handled.
…ifferent than expected

This commit solves a problem where, if adding a `is` and `is not` filter for the same user/shape caused the union not to work correctly.
As an example, when adding in `union` mode filter `is Point` and `is not Point`, then we expect the filter to always return the entire annotation catalog.
If new filters are added in this case, this should not change anything, as they should act as inclusive filters.
New tests are added for this case.
This commit removes a bug that caused an image to appear when no annotations where loaded through filters.
It also removes the now useless reference to the filteringTab from the largoContainer
@dbrembilla dbrembilla requested a review from mzur April 7, 2025 12:31
Copy link
Member

@mzur mzur left a comment

Choose a reason for hiding this comment

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

I think I found another case with flickering thumbnails but it is quite hard to reproduce. I'm quite sure I repeated the steps that caused the flickering but the second time it didn't happen...

Maybe you can have another look at possible race conditions?

This commit refactors the filtering tab and the largoContainer so that activeFilters and union values are not managed by the tab, rather they are now a prop passed by the tab to the tab.
The filtering tab now passes on the events by the annotationFilter to the largoContainer, which now manages it.
This should clarify the hierarchy of elements, reduce the number of repeated lines and allow the largoContainer the capacity of handling the filters directly in case of events.
@dbrembilla
Copy link
Contributor Author

@mzur I have not been able to reproduce the flickering... While the page is loading, it should not display anything, so I don't think race conditions play a role here.

@dbrembilla dbrembilla requested a review from mzur April 17, 2025 09:35
Copy link
Member

@mzur mzur left a comment

Choose a reason for hiding this comment

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

I found the flickering issue! Steps:

  1. Add a filter rule that matches annotations of two different labels.
  2. Select first one label, then the other to cache the filtered annotations.
  3. Add another filter rule that still matches annotations of both labels.
  4. Select the label that is currently not selected. Some patches flicker.
Screencast.from.2025-04-24.15-32-55.webm

@dbrembilla dbrembilla requested a review from mzur April 24, 2025 16:54
Copy link
Member

@mzur mzur left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for all the work 👍

@mzur mzur merged commit a8b4df5 into master Apr 25, 2025
2 checks passed
@mzur mzur deleted the adds-shape-filter branch April 25, 2025 07:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants