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

Filtering improvements #182

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Filtering improvements #182

wants to merge 28 commits into from

Conversation

bnmajor
Copy link
Collaborator

@bnmajor bnmajor commented Jan 24, 2025

  • Fix review mode toggle sort order
  • Fix some bugs with the comparison filters
    • Fix comparison operator to ensure we're comparing user ids
    • Do not disable the "any" option for second comparison choice
    • Fix clashing component ids
  • Update the groupBy options
    • Adds the option to group by prediction or by review
    • Removes the agree/disagree grouping option as it's too ambiguous
  • Update the sortBy options
    • Add the options to sort by predictions or reviews
    • Remove the option to sort by agree/disagree because of ambiguity
  • Add simple option to filter by labeled superpixels
  • Add an "All Labels" option to the labels filter menu. Selecting the "All Labels" option selects all labels. De-selecting removes all label filters. Labels can still be selected/removed individually. The "no label" filter remains its own option.
  • Update the reviews filter menu to better match the labels filter menu. This adds "All Reviews" to the reviews filter menu to mirror the setup of the labels filter menu.

@bnmajor bnmajor requested a review from manthey January 24, 2025 19:43
@manthey
Copy link
Contributor

manthey commented Jan 28, 2025

Recommendation: Show the "All Labels" as unchecked or in the ambiguous state if any of the labels are unchecked. I think. it is more likely you want to go back to showing all labels than showing no labels.

@manthey
Copy link
Contributor

manthey commented Jan 28, 2025

I'm having trouble getting the filter by comparison section to do what I expect. I want to show where predictions differ from labels (or labels differ from predictions). I can't seem to pick labels on the left. When I pick predictions and differs from, picking labels on the right seems to go back to saying any. I'm not sure what is happening:

filterby.mp4

@bnmajor bnmajor force-pushed the filtering-improvements branch from 028d720 to 49fcd10 Compare January 29, 2025 20:50
@bnmajor
Copy link
Collaborator Author

bnmajor commented Jan 29, 2025

Recommendation: Show the "All Labels" as unchecked or in the ambiguous state if any of the labels are unchecked. I think. it is more likely you want to go back to showing all labels than showing no labels.

Updated!

I'm having trouble getting the filter by comparison section to do what I expect. I want to show where predictions differ from labels (or labels differ from predictions). I can't seem to pick labels on the left. When I pick predictions and differs from, picking labels on the right seems to go back to saying any. I'm not sure what is happening:

This has been fixed now.

@manthey
Copy link
Contributor

manthey commented Feb 3, 2025

Either there is still an issue or I am confused about what is supposed to be happening. See video.

The left side of filter by only lets me pick predictions (not labels). Filter by Labels by me gets different results than filtering by labels (even though I'm the only who has labeled something); and picking labels on the right then shows "Any".

filterby.mp4

Brianna Major and others added 12 commits February 10, 2025 13:31
- Fix comparison operator to ensure we're comparing user ids
- Do not disable the "any" option for second comparison choice
- Fix clashing component ids
- Adds the option to group by prediction or by review.
- Removes the "agree/disagree" grouping option as it's too ambiguous
- Add the options to sort by predictions or reviews
- Remove the option to sort by agree/disagree because of ambiguity
Add "All Labels" option to the labels filter menu. Checking the "All Labels"
option selects all labels. Unchecking removes all label filters. Labels can
still be checked/unchecked individually. The "no label" filter remains its own
option.
This adds "All Reviews" to the reviews filter menu to mirror the setup of the
labels filter menu.
For the comparison filters make sure that we confirm that the first option and
operator have been set before trying to update any filtering.
- Improve naming for added clarity
- Add clarifying comments
- Update metadata on queued superpixels to ensure none are missed
Only disable an option if it is the exact same option as the one already
selected. Disable the second selection until the first selection and operator
are chosen.
@bnmajor bnmajor force-pushed the filtering-improvements branch from 49fcd10 to 412427e Compare February 11, 2025 13:42
Add a utility function to check whether or not a value is a valid number: not
NaN, not null, and not undefined.
@manthey
Copy link
Contributor

manthey commented Feb 17, 2025

I think we need the same fix for selecting option items rather than text for the differs from selector. Also, we should probably make the entire row of every filter box toggle the checkbox (e.g., the All Labels field should be clickable anywhere, not just on its text).

@manthey
Copy link
Contributor

manthey commented Feb 17, 2025

My test data set apparently has some superpixels without any "labeller" associated with them. I don't know if this is the cause, but I still have an issue when I select the Predictions differ from Admin (it continues to show differs from Any). If this set should be blank (or, in general, if we have filtered everything away), we should put some indicator that there is nothing selected rather than just leave the field blank.

Do not try to compute the number of superpixels to show unless there is at
least one available to fetch the size from.
This adds a tiny bit more margin so that selected superpixel highlight is not
covered up.
This allows finding ALL matching/differing labels and reviews in addition to
user labels and user reviews.
This re-write fixes some bugs that were present as well as improves readability
and adds clarifying comments.
Sets a default return value in case no fill color is found. This prevents a
console error from a race condition when the grouping selection is changed but
internally the actual grouping has not changed - this request a category fill
color for "data" which is not actually one of the user created labels.
If the component is never mounted the watcher will never be reached.
- Do not call the annotation metadata endpoint if the annotation was saved.
  Saving the annotation saves the metadata so this is an extra call for no
  reason. Only explictly save the metadata for review changes since those do
  not trigger an annotation save.
- Include a separate endpoint that is used to double-check that any labels that
  are created are not missing their associated "labeler" (user) information.
- Since we are now relying on the annotation save as a way to update the
  metadata as well, make sure calls to updateMetadata are made *before*
  saveAnnotations.
- Cleans up a few places where the new label value being passed in to be saved
  as metadata was incorrect.
- Duplicate values in filterBy list were breaking the labels and reviews menus
  because selections were not being removed as expected
- Do not include superpixels with cleared reviews when filtering by reviewer
- Fix typo that prevented filterByComparison menus from displaying correct
  selection
- Fix filterByComaprison's conversion of value to label to make sure comparison
  values are correct
- Remove unused code
- Automaically close multi-select menus on blur
@bnmajor bnmajor force-pushed the filtering-improvements branch from 56da328 to 149cea5 Compare February 28, 2025 19:18
This function was originally added to make sure that any labels that were
missing their labeler user id were fixed before the metadata was saved. After
all of the fixes we've now made this should no longer be needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants