-
Notifications
You must be signed in to change notification settings - Fork 65
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
Localizing Notebooks - continued #16
Conversation
particularly: jupyter#10 (comment) work in progress.
<a href="#">{{ _("New Notebook") }}</a> | ||
``` | ||
|
||
### Javascript files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this section is accurate - it looks like it's left over from when the proposal was to turn all JS files into templates, which we agreed we don't want to do.
Updated proposal for translation of Jupyter notebook. Please review/approve, as I'm able to work on this now that I'm back on the job. |
I'm certainly in favor of pressing forward. Let's see how it plays out. |
I agree with Kyle. The best way to vet the approach is to try it out in a few PRs against notebook and adjust accordingly. As for the proposal document, I took a quick read through. I want to go back and make a few minor suggestions. |
|
||
## Detailed Explanation | ||
|
||
The Jupyter notebook code presents a significant challenge in terms of enablement for translation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A link to https://github.com/jupyter/notebook over the text "Jupyter notebook code" might help clarify that the document is explicitly talking about the classic notebook UI and not Jupyter Lab. Given the differences between the two implementations, I think it wise to focus on the former (if that is indeed your goal) and, based on lessons learned from that, tackle Lab localization as a separate task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, Peter. I added these links and a scope section that makes it clear that this proposal is only for classic notebook. See 6c00c53
mostly because there are multiple different types of source code from which translatable UI strings | ||
are derived. | ||
|
||
In Jupyter, translatable strings can come from one of three places: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"In Jupyter Notebook"
``` | ||
|
||
Once this is complete, any calls to `_()` in the code will retrieve a translated string from | ||
${base_dir}/locale/**xx**/LC_MESSAGES/notebook.mo, where **xx** is the language code in use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming the locales will ship with notebook, not be external packages that users install. If so, it would be good to clarify this by defining base_dir
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. See bff04d5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think allowing to ship translation as external package would be good. Otherwise releases might get hold on by missing translations. I think trying to pull that from notebook-localisation
(or similar), or even notebook-i18n-lang
would be good for end-user translator to be able to work without having to install dev with node and all this stuff.
But that can be fixed later. I'm happy to leave that as-is, and modify this after the fact if we figure out how to implement it.
### Javascript files | ||
|
||
For Javascript, there are no established APIs that can consume a compiled `.mo` file directly in the same way as gettext(). | ||
However, there is a library called [Jed](https://slexaxton.github.io/Jed/) that provides an API set similar to gettext(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Jed compatible with webpack and/or requirejs (both of which are used in Jupyter Notebook)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm no javascript guru, but I think I would be, since the example on its main page shows using it as an AMD module.
|
||
|
||
## Prototype - Proof of Concept | ||
I have created a **VERY** preliminary prototype of Jupyter notebook at (https://github.com/JCEmmons/notebook/tree/intl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, awesome! 🍰
limited to classic notebook and not Jupyter lab
Picking this up after the holiday. The only thing I have left is that this proposal is scoped only to "notebook classic" as opposed to JupyterLab, and I think the reverse makes a bit more sense. JupyterLab is the future of the notebook client, and the notebook package is heading for retirement pretty soon. Plus, the discussion of tooling sounds like it will fit better in the existing JupyterLab pipeline than the notebook classic one. What do people think about that? |
Thanks, and while I understand that Jupyter lab is the future, "classic" is the present at least for us. I'm close to being ready for a preliminary PR on this, so I will welcome your comments and advice. |
As long as @JCEmmons doesn't mind the fact that there may be a bit of work to be re-done for JLab, I'm +1 here. In fact, there may be some advantage in doing it in Classic so we can kick the tires of the process atop a fairly stable and simpler codebase. Lab is more complex and moving rapidly, so I suspect doing this in Lab right now could prove quite tricky. @JCEmmons, since you seem OK with that, I'd say let's go ahead and see what we learn! @minrk pinged the steering council (thanks!) for feedback, but we'll treat silence as acquiescence if we don't hear back from the rest in a few days. No need to stall you here, and very sorry for the delay. We really appreciate your contribution, and this is an important problem we've long failed to tackle. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, we definitely need movement on this front. I've picked up a few specific things inline.
|
||
There is currently no standard approach for translating the GUI of [Jupyter notebook]( https://github.com/jupyter/notebook). | ||
This has driven some people to do a | ||
[single language translation for Jupyter 4.1](https://twitter.com/Mbussonn/status/685870031247400960). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be wrong, but I think @Carreau's tweet was pointing to a translation of the release announcement, not the UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. I often translate at least the release announce in French.
the string doesn't exist in the catalog, then the string passed as the argument to _() is | ||
returned. | ||
|
||
In this context, **${base_dir}** refers to the base installation directory for notebook. We are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definition is not entirely clear to me - I can think of a couple of directories that may be called the 'base installation directory'. I'm guessing you mean the notebook package directory (which contains, for instance, notebookapp.py
).
can be consumed by Jed. The author of [Jed](https://slexaxton.github.io/Jed/) recommends | ||
[po2json](https://www.npmjs.com/package/po2json), which can be used | ||
either as a command line utility, or directly from Javascript. I suspect that the conversion from `.po` to either | ||
`.mo` for Python or Jinja2, or conversion to JSON for Javascript would be something we would want to do at install time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably at build time to be precise - i.e. at least the wheel packages will contain pre-built .mo
and .json
translation files.
var i18n = new Jed(nbjson); | ||
var _ = function (text) { | ||
return i18n.gettext(text); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this have to be done before any JS code uses translatable strings? If all UI construction has to wait for an asynchronous request to get the JSON file, I can see that causing problems. Maybe we can stick the JSON data into the page when we fill the template to avoid that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with caching it should be ok, as we can query the file by its hash and make it expire in 99years. It depends on the size though.
|
||
[Babel](http://babel.pocoo.org/en/latest/) allows us to define a set of rules that define | ||
where all the extractable strings are (whether Python, HTML template, or Javascript), what the | ||
extraction methods are (i.e. `_()` for Python or Javascript, and `<% trans >`/`<% endtrans %>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<>
-> {}
, based on the Jinja section above
Great - you can view the work in progress on this item at
https://github.com/JCEmmons/notebook/tree/jep16. I am hoping to have a
preliminary PR ready soon.
Regards,
John C. Emmons
Globalization Architect & Unicode CLDR TC Chairman
IBM Software Group
Internet: emmo@us.ibm.com
From: Kyle Kelley <notifications@github.com>
To: jupyter/enhancement-proposals
<enhancement-proposals@noreply.github.com>
Cc: John Emmons/Austin/IBM@IBMUS, Mention <mention@noreply.github.com>
Date: 01/25/2017 11:44 PM
Subject: Re: [jupyter/enhancement-proposals] Localizing Notebooks -
continued (#16)
@rgbkrk approved this pull request.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
I spotted a few minor things that I think should be fixed (see above). Once those are dealt with, I think we can merge this. |
whenever the code base changes. The `.pot` file can be thought of as the source from which all translations are derived. | ||
|
||
Translators and/or interested contributors can then use utilities such as [Poedit](https://poedit.net/) to create | ||
translated `.po` files from the master `.pot` file for the desired languages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did experiment with https://www.transifex.com/ipython/ipython-notebook/ which is online, and we hoped at some point to integrate this into the workflow.
|
||
At install time, convert the translated `.po` into two runtime formats: | ||
* Convert to `.mo` which can be used by Python code using the gettext() APIs in Python, and can also be used by | ||
the i18n extensions in [Jinja2](http://jinja.pocoo.org/docs/dev/extensions/#i18n-extension) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by "install", does this require code execution ? If so we should find an alternative way, as wheels cannot execute code at install time.
|
||
In [Jupyter notebook]( https://github.com/jupyter/notebook), translatable strings can come from one of three places: | ||
|
||
1. Directly from Python code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicking, and not really important to address here, but we might need to clarify that this will not/cannot address translations of message coming from the kernel. IE ?
/??
will likely not be translated.
``` | ||
|
||
Once this is complete, any calls to `_()` in the code will retrieve a translated string from | ||
${base_dir}/locale/**xx**/LC_MESSAGES/notebook.mo, where **xx** is the language code in use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think allowing to ship translation as external package would be good. Otherwise releases might get hold on by missing translations. I think trying to pull that from notebook-localisation
(or similar), or even notebook-i18n-lang
would be good for end-user translator to be able to work without having to install dev with node and all this stuff.
But that can be fixed later. I'm happy to leave that as-is, and modify this after the fact if we figure out how to implement it.
var i18n = new Jed(nbjson); | ||
var _ = function (text) { | ||
return i18n.gettext(text); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with caching it should be ok, as we can query the file by its hash and make it expire in 99years. It depends on the size though.
Sorry for the delay. Minor comments none of them blocking this. One other point about translating string I haven't seen is the use of |
One more thing to think about : the notebook might not be the only package we need to localize for the UI to be fully localized. |
There is a related proposal for bi-directional support in the notebook: |
Hi @srl295 👋—Zach from the @jupyter/software-steering-council here. We're working through old JEPs and closing proposals that are no longer active or may not be relevant anymore. Under Jupyter's new governance model, we have an active Software Steering Council who reviews JEPs weekly. We are catching up on the backlog now. Since there has been no active discussion on this JEP in awhile, I'd propose we close it here (we'll leave it open for two more weeks in case you'd like to revive the conversation). If you would like to re-open the discussion after we close it, you are welcome to do that too. Keep in mind that JupyterLab (and Notebook v7) now includes a fairly robust internalization+localization system. If you have additional ideas to expand this system, free free to comment here anytime. |
Continuation of: #10
Based on @TwistedHardware ’s commits. (I dropped the ones which modified the base template.)