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

Support for Chinese and other UTF8 characters as folder names #5634

Closed
wants to merge 2 commits into from

Conversation

Littleor
Copy link

  • Motivation for features / changes

When the folder is named Chinese and other UTF8 characters, it is not possible to toggle Runs on the front end.

As shown below, the toggle will not work because of the btoa function.

Screen Shot 2022-03-29 at 19 45 41

  • Technical description of changes

The encoding method of the characters before atob has been added to avoid the above problem.

  • Screenshots of UI changes

None

  • Detailed steps to verify changes work correctly (as executed by you)

Create a folder called 中文字符 in the log path and store ri in it, then start tensorboard, toggle Runs in the lower left corner of SCALARS does not work and the console reports an error (see above)

  • Alternate designs / implementations considered

None

@google-cla
Copy link

google-cla bot commented Mar 29, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@bmd3k bmd3k self-requested a review April 19, 2022 14:18
@bmd3k
Copy link
Contributor

bmd3k commented May 17, 2022

First off, apologies for taking so long to respond. This has been on the back of my mind but I am just getting to it. Thanks for your contribution to TensorBoard!

Have you used the "Time Series" dashboard? It is a new, better-supported dashboard that includes Scalar charts. We have recently elevated it to the first dashboard in the nightly builds (See #5704 for more details). It does not have the problem you are seeing in the Scalars dashboard but has the downside that run selection is not currently stored in the URL. Would it be sufficient for you to start using the "Time Series" dashboard instead?

As for this change, there are a couple concerns that I have that I want to think about or explore further:

  1. We would want to ensure backward compatibility with the existing implementation to make sure users' bookmarked URLs still work. I have only done a little bit of testing here so far.
  2. I hesitate to rely on unescape() to solve the problem given its semi-deprecated state and warnings of undesirable behavior (as descrbied in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/unescape).

So I await your response to the suggestion to use the Time Series dashboard and then we can decide if we need to continue to consider this change.

@bmd3k bmd3k closed this Feb 10, 2023
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.

None yet

2 participants