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

Adds hparam filterbar to runs table #6692

Merged
merged 6 commits into from
Dec 7, 2023

Conversation

hoonji
Copy link
Member

@hoonji hoonji commented Dec 4, 2023

Motivation for features / changes

To view applied hparam filters in the runs table.

Technical description of changes

If a user filters the runs table based on a hparam column, the filter will be shown as a chip above the table. Clicking the chip will allow the user to modify the filter via a dialog. Clicking the close button on the chip will remove the filter.

Screenshots of UI changes (or N/A)

Screenshot 2023-12-04 at 18 30 27

@hoonji
Copy link
Member Author

hoonji commented Dec 4, 2023

@JamesHollyer The cl requires some minor changes (need to add icon once I get tf_mirror permissions), but the other parts are ready for review. This is my first major PR on this code base - please be as critical as necessary!

Copy link
Contributor

@JamesHollyer JamesHollyer left a comment

Choose a reason for hiding this comment

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

This all looks good. I am only not approving as I want to see the finished PR with the correct icon.

rileyajones added a commit that referenced this pull request Dec 5, 2023
## Motivation for features / changes
I followed my own instructions documented in #6692
I did encounter an issue with the tf mirror cli but I managed to work
around it.
@hoonji
Copy link
Member Author

hoonji commented Dec 5, 2023

All changes applied (including the icon)! PTAL.

Copy link
Contributor

@JamesHollyer JamesHollyer left a comment

Choose a reason for hiding this comment

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

LGTM.

Please re-request the review when appropriate.
Screenshot 2023-12-06 at 10 30 40 AM

@hoonji
Copy link
Member Author

hoonji commented Dec 7, 2023

Right, I was actually looking for something like that. Thanks!

@hoonji hoonji merged commit 4431538 into tensorflow:master Dec 7, 2023
13 checks passed
nfelt pushed a commit that referenced this pull request Dec 7, 2023
## Motivation for features / changes
Copybara sync will complain after #6692 :
- Filterbar doesn't use lowerCamelCase in property names (it used a "_"
prefix)
- runs table BUILD target doesn't include chip dependencies
(surprisingly this doesn't affect tests or app execution)

This PR addresses both issues
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