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

vz-example-viewer: fix int feature list type #2266

Merged
merged 3 commits into from
May 23, 2019

Conversation

wchargin
Copy link
Contributor

@wchargin wchargin commented May 22, 2019

Summary:
Caught during a Clutz upgrade. From context, it looks like ints should
be an Int64List, as is already the case in parseFeature below.

Test Plan:
This preserves existing behavior. To exercise the codepath, run

bazel run //tensorboard/plugins/interactive_inference/tf_interactive_inference_dashboard/demo:agedemoserver

then navigate to /tf-interactive-inference-dashboard/age_demo.html in
a browser. Select a data point in the plot. In the list of features for
that data point (on the left side of the screen), click the “+” button
immediately below the bottom feature. Add an Int feature, and set its
value to some number. Select a different data point, then select the
first data point again, and observe that the new feature and value are
still present.

wchargin-branch: vzev-ints-list-type

Summary:
Caught during a Clutz upgrade. From context, it looks like `ints` should
be an `Int64List`, as is already the case in `parseFeature` below.

Test Plan:
Fingers crossed; I’m not familiar with this part of the codebase.

wchargin-branch: vzev-ints-list-type
@wchargin
Copy link
Contributor Author

@jameswex: How should this be tested?

@jameswex
Copy link
Contributor

I tested this change locally by running a WIT demo, adding a new int feature in the UI, setting it to an int value, clicking away to another example, then back to the changed example and seeing the value correctly persisted.

@wchargin
Copy link
Contributor Author

Thanks!

I tested this change locally by running a WIT demo, adding a new int
feature in the UI, setting it to an int value, clicking away to
another example, then back to the changed example and seeing the value
correctly persisted.

I did this (I think), but when I set the value to 1234.5, clicked a
different data point, and clicked back, the value was still 1234.5
(rather than being truncated to 1234 or left absent). This occurs both
before and after this change. Is this intended?

Is there an observable difference before vs. after this change, or is it
just hygiene?

wchargin-branch: vzev-ints-list-type
wchargin-source: baaac799b078f2ef007af6353bedb685f6cca2e6
wchargin-branch: vzev-ints-list-type
@jameswex
Copy link
Contributor

hygiene as since the example is passed from js back to python as json, this never exhibited any outwardly-bad behavior from what i can tell.

@wchargin
Copy link
Contributor Author

Understood; thanks.

@wchargin wchargin merged commit 555236a into master May 23, 2019
@wchargin wchargin deleted the wchargin-vzev-ints-list-type branch May 23, 2019 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants