Fix PD plots in py3 #2669
Merged
Fix PD plots in py3 #2669
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@tolga-b please review |
tolga-b
suggested changes
Sep 19, 2019
| """Inits OriginalFeatureList.""" | ||
| self.feature_name = feature_name | ||
| self.original_value = original_value | ||
| self.original_value = [ |
There was a problem hiding this comment.
I think it's better to use ensure_text from six for consistency with #2311.
There was a problem hiding this comment.
actually can't as tensorboard/tensorflow depend on six 1.10.0 which doesn't include the ensure_* functions. our separate witwidget code seems to pull in six 1.12.0 due to transitive dependencies that is why #2311 works for us. so i reverted back to the first implementation and added an explicit dependency in witwidget setup.py on six 1.12.0 to improve the pip package setup dependencies.
tolga-b
approved these changes
Sep 20, 2019
stephanwlee
approved these changes
Sep 20, 2019
| """Inits OriginalFeatureList.""" | ||
| self.feature_name = feature_name | ||
| self.original_value = original_value | ||
| self.original_value = [ensure_not_binary(v) for v in original_value] |
There was a problem hiding this comment.
v -> human readable name please :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for features / changes
PD plots didn't work at all in notebook mode with py3 kernels.
Technical description of changes
Ensure string feature values used in PD plot generation are sent to JS as strings for json serialization, not as bytes, to avoid failing serialization.
Correctly use the bytes when running in py3 as that is what tf.Example expects for bytes_list, whereas in py2 it functions on strings.
Make use of six.bytes_type and encode and decode functions to correctly handle this.
Ran notebooks in py2 and py3 with this change and verified that PD plots are shown correctly (with correct values as well) for both numeric and categorical features.