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

Introduce tf-per-run-steps-selector #471

Closed
wants to merge 2 commits into from

Conversation

chihuahua
Copy link
Member

@chihuahua chihuahua commented Sep 4, 2017

The PR curves plugin had used a special tf-pr-curve-steps-selector
component for letting the user select a single step to apply across all
tags for a given run. Other plugins could also benefit from that
functionality. See #469.

This change moves that tf-pr-curve-steps-selector component into the
components directory and renames it to tf-per-run-steps-selector. Some
documentation was added to several properties of the component.

Also removed the tf-color-scale import from tf-pr-curve-dashboard. The
dashboard does not need it.

The PR curves dashboard WAI.

npyuxfpczel

@chihuahua chihuahua requested review from jart and wchargin September 4, 2017 02:07
The PR curves plugin had used a special tf-pr-curve-steps-selector
component for letting the user select a single step to apply across all
tags for a given run.

This change moves that tf-pr-curve-steps-selector component into the
components directory and renames it to tf-global-steps-selector. Some
documentation was added to several properties of the component.

Also removed the tf-color-scale import from tf-pr-curve-dashboard. The
dashboard does not need it.
@wchargin
Copy link
Contributor

wchargin commented Sep 4, 2017

This doesn't sound like what folks are asking for in #469. It isn't a "global step slider for images," it's one slider per run.

@chihuahua chihuahua changed the title Introduce tf-global-steps-selector Introduce tf-per-run-steps-selector Sep 4, 2017
@chihuahua
Copy link
Member Author

True. How about tf-per-run-steps-selector?

@wchargin
Copy link
Contributor

wchargin commented Sep 5, 2017

I mean, yeah, that's a better name, but take a step back: this isn't what users want. (No one has requested this; people have requested a single global slider.) Why do you want to implement it?

@chihuahua
Copy link
Member Author

A single global slider seems sub-optimal (but a potential option - It is more convenient.) because runs can differ by a lot in steps, whereas steps are often the same within a run. What do you think?

@wchargin
Copy link
Contributor

wchargin commented Sep 5, 2017

Asked in #469.

@chihuahua
Copy link
Member Author

Closing in favor of a global step slider (see my latest comment in #469) , although I still prefer that we unify and recommend 1 component for step selection in the future. That gets us code reuse (for plugin developers) and UI consistency (for end users).

Really though, what I really want right now is just to try something reasonably justified and see how users react.

@chihuahua chihuahua closed this Sep 5, 2017
@chihuahua chihuahua deleted the chihuahua-generalize-slider branch September 7, 2017 22:16
@chihuahua chihuahua restored the chihuahua-generalize-slider branch September 7, 2017 22:16
@chihuahua chihuahua deleted the chihuahua-generalize-slider branch September 10, 2017 03:21
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