KEMBAR78
[tensor-widget] Add colormap selection and Ctrl/Alt/Shift+wheel zooming by caisq · Pull Request #2764 · tensorflow/tensorboard · GitHub
Skip to content

[tensor-widget] Add colormap selection and Ctrl/Alt/Shift+wheel zooming #2764

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 22, 2019
Merged

[tensor-widget] Add colormap selection and Ctrl/Alt/Shift+wheel zooming #2764

merged 6 commits into from
Oct 22, 2019

Conversation

@caisq
Copy link
Contributor

@caisq caisq commented Oct 11, 2019 •
edited
Loading

  • Motivation for features / changes

    • Continue developing TensorWidget

  • Technical description of changes

    • Support Ctrl/Alt/Shift + mouse wheel zooming
    • Add an optional field to TensorWidgetOptions: wheelZoomKey to allow specifying whether alt, ctrl or shift key activates wheel zooming.
    • Add jet colormap
    • Add colormap selection menu
    • Added a color bar in the value tooltip that indicates where the currently pointed-to value is within the color scale.

  • Screenshots of UI changes

    • image
    • image
    • image
    • image

@wchargin
Copy link
Contributor

In the rest of TensorBoard we use Alt+Wheel for zooming to avoid
conflicting with standard browser interfaces. Maybe consider following
that precedent here?

@caisq
Copy link
Contributor Author

caisq commented Oct 15, 2019

@wchargin Thanks for the info. I added an optional field to TensorWidgetOptions: wheelZoomKey to allow specifying whether alt, ctrl or shift key activates wheel zooming.

@caisq caisq requested review from bileschi and wchargin October 15, 2019 01:35
@caisq caisq changed the title [tensor-widget] Add colormap selection and Ctrl+wheel zooming [tensor-widget] Add colormap selection and Ctrl/Alt/Shift+wheel zooming Oct 20, 2019
@caisq
Copy link
Contributor Author

caisq commented Oct 21, 2019

Gentle ping @bileschi @wchargin

@bileschi
Copy link
Collaborator

The native github review tool is very bad. Let's re-open the suggestion to use reviewable.

bileschi
bileschi reviewed Oct 21, 2019
View reviewed changes
Copy link
Collaborator

@bileschi bileschi left a comment

Choose a reason for hiding this comment

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

LGTM

tensorboard/components/tensor_widget/colormap.ts Outdated

/**
* Minimum value that the color map can map to without clipping.
* Must be a finite value and be >= `min`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens in the case of equality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then all finite values will mapped to a relative value of 0.5 and in turn be mapped to the mid-point of the colormap. Added comment here.

@wchargin wchargin removed their request for review October 21, 2019 21:08
@wchargin
Copy link
Contributor

I won’t have bandwidth to get to this shortly. Let me know if you need
anything specific and bounded.

@caisq
Copy link
Contributor Author

caisq commented Oct 22, 2019

Thanks, @bileschi and @wchargin

@caisq
Copy link
Contributor Author

caisq commented Oct 22, 2019

cc @mndevec

bileschi
bileschi approved these changes Oct 22, 2019
View reviewed changes
Copy link
Collaborator

@bileschi bileschi left a comment

Choose a reason for hiding this comment

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

approved

@caisq caisq merged commit 99e315f into tensorflow:master Oct 22, 2019
@caisq caisq deleted the tensor-widget-ctrl-wheel branch October 22, 2019 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bileschi bileschi bileschi approved these changes

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@caisq @wchargin @bileschi @googlebot