determined icon indicating copy to clipboard operation
determined copied to clipboard

feat: support filter in flat run table

Open keita-determined opened this issue 10 months ago • 5 comments

Ticket

ET-111

Description

Add filter feature in Flat Run table (Run table is under feature flag)

Test Plan

  • Enable feature flag of Flat Runs View
  • Go to flat run table
  • Check if
    • filter button is visible and usable on top of the run table
    • filter is saved after applying valid filters, so filtered table is preserved even after refreshing the page
    • filter should be able to be applied from table header
    • filter should be able to be cleared from table header (filter behavior should be the same as the experiment table we have now (its not feature flagged))

Checklist

  • [x] Changes have been manually QA'd
  • [ ] User-facing API changes need the "User-facing API Change" label.
  • [ ] Release notes should be added as a separate file under docs/release-notes/. See Release Note for details.
  • [ ] Licenses should be included for new code which was copied and/or modified from any external code.

keita-determined avatar Apr 26 '24 00:04 keita-determined

Deploy Preview for determined-ui ready!

Name Link
Latest commit 0630fad9182fd68ced95fe3f642e9d970dce909c
Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/66341ebd69f67a0008a5c684
Deploy Preview https://deploy-preview-9250--determined-ui.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Apr 26 '24 00:04 netlify[bot]

Codecov Report

Attention: Patch coverage is 0% with 151 lines in your changes are missing coverage. Please review.

Project coverage is 37.91%. Comparing base (e31135a) to head (0630fad). Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9250      +/-   ##
==========================================
- Coverage   44.60%   37.91%   -6.69%     
==========================================
  Files        1275      951     -324     
  Lines      156240   116539   -39701     
  Branches     2450     2450              
==========================================
- Hits        69687    44183   -25504     
+ Misses      86313    72116   -14197     
  Partials      240      240              
Flag Coverage Δ
harness ?
web 34.99% <0.00%> (-0.06%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
webui/react/src/components/TableActionBar.tsx 0.00% <0.00%> (ø)
...bui/react/src/pages/F_ExpList/F_ExperimentList.tsx 0.00% <0.00%> (ø)
...ui/react/src/components/FilterForm/TableFilter.tsx 0.00% <0.00%> (ø)
...ebui/react/src/pages/FlatRuns/FlatRuns.settings.ts 0.00% <0.00%> (ø)
webui/react/src/pages/FlatRuns/FlatRuns.tsx 0.00% <0.00%> (ø)

... and 324 files with indirect coverage changes

codecov[bot] avatar Apr 26 '24 00:04 codecov[bot]

  • noticed that Searcher Metrics Value does not show filter options in header menu but it does still appear in the Filters dropdown menu. This is true for Experiments List, too. But maybe worth addressing here?

fixed in run table and experiment table

  • Also, it looks like the Searcher ID and Searcher Description columns appear in the table but can't be filtered, even though they're not in BANNED_FILTER_COLUMNS (and I don't think we would want them to be?). I think we just need to make sure everything in getColumnDefs and runColumns matches. May need to update again with https://github.com/determined-ai/determined/pull/9146 but should probably make sure current columns work with filters as expected.

Searcher ID and Searcher Description filters exit. in https://github.com/determined-ai/determined/blob/e31135ae65352c776df97e58335ac133b5d37fe0/webui/react/src/pages/FlatRuns/columns.ts#L236, but the column names are overwritten, so Searcher ID filter exits as ID. Search Name filter exits as Name. Searcher Description filter exits as Description. imo these names shouldn't be overwritten in frontend. that causes the name mismatching like this.

@johnkim-det what do you think is the best solution for this? If we wanna keep the current column names, we need to fix backend. if we wanna change the column name, these should match the column data from backend. i think backend change is required for either way

  • Also not sure why, but I don't think ID filter is working. Returns no results even when I'm expecting one to be returned.

ID means run id instead of experiment id? looks like backend doesn't support it yet. I don't think frontend can do much with it afaik.

keita-determined avatar May 02 '24 23:05 keita-determined

yeah I think it makes sense to always use the names from the backend -- might be better to read the names from projectColumns in getColumnDefs instead of using title field.

But I'm less concerned with the mismatch of column names between filters menu and column headers than the fact that the column header menus aren't displaying filter options at all for those columns. the name mismatch shouldn't cause that, right?

johnkim-det avatar May 03 '24 18:05 johnkim-det

But I'm less concerned with the mismatch of column names between filters menu and column headers than the fact that the column header menus aren't displaying filter options at all for those columns. the name mismatch shouldn't cause that, right?

Right, the name mismatch doesnt cause the issue. Backend doesnt return these columns. thats the issue. i dont think web cant do anything with this until backend return missing columns

keita-determined avatar May 03 '24 18:05 keita-determined

made a ticket

keita-determined avatar May 06 '24 20:05 keita-determined