Skip to content

Multiselect filtering for "no values" in nblocks column #1106

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BenjaminCharmes
Copy link
Contributor

Closes #1104

Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.28%. Comparing base (ccd933c) to head (1b06ef9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1106   +/-   ##
=======================================
  Coverage   70.28%   70.28%           
=======================================
  Files          63       63           
  Lines        4129     4129           
=======================================
  Hits         2902     2902           
  Misses       1227     1227           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

cypress bot commented Apr 9, 2025

datalab    Run #3069

Run Properties:  status check passed Passed #3069  •  git commit b511e749f8 ℹ️: Merge 1b06ef98cc66ee07a8ebc3c41e66b084341c9a73 into ccd933c60222ee1e124b5409f7f6...
Project datalab
Branch Review bc/filter-noblock
Run status status check passed Passed #3069
Run duration 07m 31s
Commit git commit b511e749f8 ℹ️: Merge 1b06ef98cc66ee07a8ebc3c41e66b084341c9a73 into ccd933c60222ee1e124b5409f7f6...
Committer Ben Charmes
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 477
View all changes introduced in this branch ↗︎

@jdbocarsly
Copy link
Member

I haven't tried on this branch, but I was also noticing that "match any" seems to always match samples with 0 blocks. Is that easy to fix?

@ml-evs
Copy link
Member

ml-evs commented Apr 9, 2025

This is doing something a bit strange on my local instance, filtering for "match all" -> "no blocks" still leaves some entries that have blocks. Perhaps relatedly, on main and this PR, if I try to simply sort in ascending order by the blocks column, it starts with the entries with 1 block rather than 0.

@BenjaminCharmes
Copy link
Contributor Author

BenjaminCharmes commented Apr 10, 2025

Thank for the review @jdbocarsly & @ml-evs !

  • For Josh, do you mean with "Math any" and the second dropdown empty ? (With just "any" and no blocks selected ?). If yes, now that we add "No blocks" in the dropdown, I guess it make sense that "Match any" also give sample with no block ? But yes, before, it should only have filter samples with blocks.
  • I can't reproduce the first behaviour Matt describes with "match all -> no blocks"
  • For the second behaviour you describe, it's probably because the box is empty (and doesn't have ‘0’) when there are no blocks. I can look for a way to change that

I'm going to take a closer look at all this!

@ml-evs
Copy link
Member

ml-evs commented May 2, 2025

Realised what is causing this behaviour: when you copy a sample that has blocks, the block is copied identically and then the API summary doesn't include it even though nblocks: 1 -- weird bug we need to fix

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.

Possibility of filtering multiselect for "no values"
3 participants