Date and number localization #2207

alexcjohnson · 2017-12-15T13:05:47Z

Adds date and number localization to the string localization framework of Localization #2195
Uses d3.locale format
Pulls in and cleans up all the information we already have in world-calendars
- None of these localizations are built into the bundle, but they are provided as requireable modules in lib and as standalone scripts in dist, as started by Localization #2195.
- Does NOT provide localizations for non-gregorian calendars. world-calendars does have a few of these, but mostly these calendars are only in use in one region and are already localized to this region. If there is a need we can address this later.
- Some required information for d3.locale is not available in world-calendars: AM/PM translations (the period field), dateTime and time format fields (only relevant for custom formats anyway so this doesn't seem like a big deal) and, most importantly, number formatting (decimal and thousands, we only have partial support for groupings and currency right now but they should be included too if they're different from English). Now that we have these localization skeletons we can encourage community members to fill in the rest, and even add more.
Fixes hoverformat/tickformat overwrites separators #1264 - and incidentally (as noted in a TODO) in V2 we could consider removing layout.separators altogether and only using locales.

alexcjohnson · 2017-12-15T15:41:00Z

src/core.js

+exports.register([
+ require('./locale-en'),
+ require('./locale-en-US')
+]);


🎩 bundle_tests/core_test 🌟

Since we're constructing our own d3.locale now no matter what, locale-en needs to be part of the core so it can be used as the fallback for missing pieces of other locales. Then for simplicity I moved the default locale-en-US to the core as well, which has the nice side-effect of keeping these redundant locales out of dist/.

@etpinard seem reasonable? Are you OK with putting these two in the root src/ directory? At least as long as we're not going to put any other locales into the core it doesn't seem like they warrant their own directory.

Minor downside is the lib/index* files don't demonstrate how to register locales, but we should be able to to better than documenting it there anyhow - somewhere we will want examples of how to use locales both in the build process and via script tags.

@etpinard seem reasonable?

This is very reasonable 👌

but we should be able to to better than documenting it there anyhow

Yep, we should at least open up an issue on https://github.com/plotly/documentation/issues and / or add a Locale section to dist/README.md via tasks/stats.js.

updated dist/README.md -> 331b2da

will open an issue at https://github.com/plotly/documentation once this is merged.

alexcjohnson · 2017-12-15T15:53:10Z

src/lib/dates.js

+var yearFormatD3 = '%Y';
+var monthFormatD3 = '%b %Y';
+var dayFormatD3 = '%b %-d';
+var yearMonthDayFormatD3 = '%b %-d, %Y';


This is for our default, adaptive date labels. I'm not too worried about it, as these formats are at least unambiguous. But it shouldn't be too hard to allow these to be localized too, essentially as an extension to the d3.locale format objects, should our international users feel strongly about it (and contribute their own alternative format strings).

alexcjohnson · 2017-12-15T15:55:19Z

src/plot_api/plot_config.js

+ // Unlike d3.locale, every key is optional, we will fall back on English ('en').
+ // Currently `grouping` and `currency` are ignored for our automatic number
+ // formatting, but can be used in custom formats.
+ formats: {}


I suppose we could combine these into one, so context looks more like the register-able locale modules. I didn't do that because I wanted to preserve compatibility with #2195 but that hasn't been released to npm yet so we can change it if we want. Thoughts?

So, dictionaries + formats would become locale as config argument?

If so, I'd vote 👍 for consistency with what Plotly.register expects.

... and don't worry about breaking things that haven't been released 😉

combined dictionaries + formats -> locales in 27037af

alexcjohnson · 2017-12-15T15:56:42Z

src/plots/cartesian/set_convert.js

@@ -447,9 +447,19 @@ module.exports = function setConvert(ax, fullLayout) {
 ax._min = [];
 ax._max = [];

- // copy ref to fullLayout.separators so that
+ // Fropagate localization into the axis so that


I love to Fropagate whenever I can ;)

alexcjohnson · 2017-12-15T16:04:41Z

src/core.js

@@ -68,7 +68,7 @@ exports.register([
 // locales en and en-US are required for default behavior
 exports.register([
 require('./locale-en'),
- require('./locale-en-US')
+ require('./locale-en-us')


how did this not fail for me locally but only on CI?

Added a check for case correctness of all the requires 43b73e0

etpinard

Looking great. 🎉

Using d3.locale is definitively the cleanest way to do this. Its v4 module d3-time-format doesn't look too bloated, so this looks like the right way going forward even when we'll update to d3 v4.

Fixes #1264 - and incidentally

I didn't see a new test case for this specifically, would you mind adding one if you haven't already?

Finally about

Now that we have these localization skeletons we can encourage community members to fill in the rest, and even add more.

I'm thinking we should make a pinned post on https://community.plot.ly/ like @chriddyp did to advertise the Dash workshops to attract potential contributors.

etpinard · 2017-12-15T16:01:13Z

src/plots/plots.js

+ locale = baseLocale;
+ }
+
+ // lastly pick out defaults from english (non-US, as DMY is so much more common)


I strongly agree with picking en (not en-us) as our default 👌

picking en (not en-us) as our default

OK, so what we're doing right now is a bit schizophrenic, but it has a certain logic to it: en-US is the default locale, but if you specify a different locale and it has an incomplete definition (After potentially combining the region and language definitions), it will fill in with non-translated strings (for which we've use US english, en-US) but en for the missing formatting fields. But the ONLY difference between en and en-US formatting is if you make a custom tickformat with %x (and possibly %c) in it - you get DD/MM/YYYY format from en vs MM/DD/YYYY from en-US. Seemed to me that since the US is about the only country in the world that uses the utterly nonsensical MM/DD/YYYY format, if you've already said you're not in the US we shouldn't fall back on that.

For strings, the only place non-translated is different is different is color/colour. I guess I don't care much about that, it currently only shows up in "Click to enter Colo(u)rscale title," and I see no legitimate reason to give an incomplete set of string translations unless you're in another english region (in which case en is part of the inheritance chain anyway so you'll get your silly extra "u")

Any quibbles with that logic?

Any quibbles with that logic?

None from me 👌

etpinard · 2017-12-15T16:01:55Z

src/plots/plots.js

+/**
+ * getFormatter: combine the final separators with the locale formatting object
+ * we pulled earlier to generate number and time formatters
+ * TODO: remove separators in v2, only use locale, so we don't need this step?


I'm in strong agreement here too 👌

Don't forget to add it #420 🚬

That said, I just remembered that someone (in #1842) has asked for per-axis separators settings. I didn't really understand that person's use case though. Personally, I think separators are better served as locale/context options, but maybe I'm wrong.

added #420 (comment)

That said, I just remembered that someone (in #1842) has asked for per-axis separators settings.

commented over there #1842 (comment)

etpinard · 2017-12-15T16:05:03Z

src/registry.js

@@ -328,23 +329,38 @@ function getTraceType(traceType) {
 * the dictionary mapping input strings to localized strings
 * generally the keys should be the literal input strings, but
 * if default translations are provided you can use any string as a key.
+ * @param {object} module.format
+ * a `d3.locale` format specifier for this locale
+ * any omitted keys we'll fall back on en-US


Hmm. Isn't the fallback en ?

to be precise, we'll fall back on no translation (which means en-US) - more detail #2207 (comment)

etpinard · 2017-12-15T16:18:54Z

tasks/pull_date_format.js

+var argLocale = args[0];
+
+var pathToEn = path.join(constants.pathToLib, 'locale-en.js');
+var pathToWCRegions = path.join(__dirname, '../node_modules/world-calendars/dist/regional');


path.join(require.resolve('world-calendars'), 'dist', 'regional')

would be a bit more robust

Neat, I didn't know about require.resolve - took a little fiddling but -> d0e0158

etpinard · 2017-12-15T16:20:07Z

tasks/pull_date_format.js

+ .replace(/'/g, '\\\'') // escape single quotes
+ .replace(/"([A-Za-z]\w+)":/g, '$1:') // unquote simple identifier keys
+ .replace(/([^\\])"/g, '$1\'') // replace unescaped doublequotes with singlequotes
+ .replace(/\\"/g, '"'); // unescape escaped doublequotes


Nasty. That most have been painful to do. 👏

etpinard · 2017-12-15T16:28:31Z

src/plots/cartesian/set_convert.js

@@ -447,7 +447,7 @@ module.exports = function setConvert(ax, fullLayout) {
 ax._min = [];
 ax._max = [];

- // Fropagate localization into the axis so that
+ // Propagate localization into the axis so that


etpinard · 2017-12-15T16:39:44Z

src/locale-en.js

+ 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun',
+ 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec'
+ ],
+ periods: ['AM', 'PM'],


I'm curious here. Do locales w/o periods set fallback to using a 24-hour clock?

periods is only relevant if you use %p (and %I for 12-hour hours) in a custom format, our standard formatting always uses a 24-hour clock. Locales that don't define periods will inherit ['AM', 'PM'] from en. Seems like most places don't care about this, and some that do are still OK with AM/PM (since it's latin anyway).

etpinard · 2017-12-15T16:43:34Z

src/traces/heatmap/hover.js

@@ -29,7 +29,6 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, hoverLay
 zmask = cd0.zmask,
 range = [trace.zmin, trace.zmax],
 zhoverformat = trace.zhoverformat,
- _separators = trace._separators,


So we probably can 🔪 these now:

🔪 trace._separators in 592fc9e

etpinard · 2017-12-15T16:48:30Z

src/plot_api/plot_config.js

+ // Unlike d3.locale, every key is optional, we will fall back on English ('en').
+ // Currently `grouping` and `currency` are ignored for our automatic number
+ // formatting, but can be used in custom formats.
+ formats: {}


So, dictionaries + formats would become locale as config argument?

If so, I'd vote 👍 for consistency with what Plotly.register expects.

etpinard · 2017-12-15T16:48:59Z

src/plot_api/plot_config.js

+ // Unlike d3.locale, every key is optional, we will fall back on English ('en').
+ // Currently `grouping` and `currency` are ignored for our automatic number
+ // formatting, but can be used in custom formats.
+ formats: {}


... and don't worry about breaking things that haven't been released 😉

in both registry (they were already combined in the API just not under the hood) and in config (where you were required to split them out yourself previously)

alexcjohnson · 2017-12-16T00:12:55Z

Fixes #1264 - and incidentally

I didn't see a new test case for this specifically

Good call -> 2a8d38f

dist will stay flat, to make script and CDN use consistent

alexcjohnson · 2017-12-18T20:58:13Z

I'm thinking we should make a pinned post on https://community.plot.ly/ like @chriddyp did to advertise the Dash workshops to attract potential contributors.

That's a great idea - wait until this is published (1.32), then start including localizations in 1.33? I'll try to get @StelaJohnson to help me fill in the dictionary for sq (Albanian) so potential contributors have a complete example to work off, @etpinard perhaps you want to do (or find someone to do) fr? Is fr-QC different enough to be noticed in plotly.js?

etpinard · 2017-12-18T21:15:17Z

Amazing PR 💃

etpinard · 2017-12-18T21:16:26Z

Don't forget to either close (or update with remaining open items) #856 after merging.

alexcjohnson added 3 commits December 15, 2017 07:45

use d3.locale formats just like the localization dicts of #2195

eacc99b

pull in all locales existing in world-calendars

fe812eb

move locales en and en-US into the core

64968dd

alexcjohnson commented Dec 15, 2017

View reviewed changes

alexcjohnson added status: reviewable feature something new labels Dec 15, 2017

🙈

a527828

alexcjohnson commented Dec 15, 2017

View reviewed changes

etpinard reviewed Dec 15, 2017

View reviewed changes

test case correctness of all require statements

43b73e0

This was referenced Dec 15, 2017

wishlist for potential breaking changes since v1 #420

Closed

Per-axis separators attribute #1842

Closed

alexcjohnson added 4 commits December 15, 2017 17:18

combine dictionaries and formats into locales

27037af

in both registry (they were already combined in the API just not under the hood) and in config (where you were required to split them out yourself previously)

explicit test for #1264

2a8d38f

use require.resolve in pull_date_format

d0e0158

🔪 unused trace._separators

592fc9e

alexcjohnson added 2 commits December 18, 2017 14:57

move lib/locale-* to lib/locales/* and update scripts accordingly

614d5be

dist will stay flat, to make script and CDN use consistent

describe localization in dist/README.md

331b2da

alexcjohnson merged commit 0dc2eb7 into master Dec 18, 2017

alexcjohnson deleted the i18n-dates branch December 18, 2017 21:24

alexcjohnson mentioned this pull request Dec 18, 2017

Provide proper i18n support #856

Closed

alexcjohnson mentioned this pull request Jan 18, 2018

Localize auto-formatted x-axis date ticks #2261

Merged

Date and number localization #2207

Date and number localization #2207

Conversation

alexcjohnson commented Dec 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcjohnson commented Dec 16, 2017 • edited Loading

alexcjohnson commented Dec 18, 2017

etpinard commented Dec 18, 2017

etpinard commented Dec 18, 2017

alexcjohnson commented Dec 16, 2017 •

edited

Loading