SEPURI-SAI-KRISHNA commented on code in PR #45410:
URL: https://github.com/apache/superset/pull/45410#discussion_r4325435817
##########
superset/charts/client_processing.py:
##########
@@ -73,6 +75,27 @@
# The Excel equivalent, applied as a cell format so the value stays numeric.
EXCEL_PERCENT_FORMAT = "0.0%"
+# "Adaptive formatting" in the time format controls, mirroring
``SMART_DATE_ID``
+# in ``superset-ui-core/time-format``. It is an alias the browser resolves to a
+# formatter that picks a representation per value, not a format string.
+SMART_DATE = "smart_date"
+
+# ``strftime`` copies a directive it does not recognize into its output
+# verbatim, so a d3-only one would reach the report as literal text: ``%L`` and
+# ``%q`` stay ``%L`` and ``%q`` rather than becoming milliseconds and a
quarter.
+# These are the directives that carry the same meaning in d3-time-format and in
+# ``strftime``. Left out are the locale-composite ones (``%c``, ``%x``,
``%X``),
+# whose output follows the server's locale rather than the chart's, and the
+# platform-dependent or d3-only ones (``%e``, ``%g``, ``%L``, ``%q``, ``%Q``,
+# ``%s``, ``%Z``, ``%z``).
+SHARED_TIME_DIRECTIVES = frozenset("aAbBdfGHIjmMpSuUVwWyY%")
Review Comment:
You are right, and I had the wrong mental model of what those five directives
depend on. I ran the chart's own formatter to be sure, `timeFormatLocale`
from
`d3-time-format` with the defaults and then with French names merged over
them
the way `TimeFormatterRegistry.setD3Format` does, against
`2024-01-15 14:30 UTC`:
| format | d3 defaults | with `D3_TIME_FORMAT` names |
| --- | --- | --- |
| `%B %Y` | `January 2024` | `janvier 2024` |
| `%b %d` | `Jan 15` | `janv. 15` |
| `%A` | `Monday` | `lundi` |
| `%a` | `Mon` | `lun.` |
| `%I:%M %p` | `02:30 PM` | `02:30 p.m.` (with `periods`) |
| `%Y-%m-%d` | `2024-01-15` | `2024-01-15` |
`strftime` gives the left column on an English worker whatever is configured,
and the worker's `LC_TIME` on a localized one, so the two agree only by
coincidence. `D3TimeFormat` in `config.py` carries exactly the five tables
behind those directives: `days`, `shortDays`, `months`, `shortMonths` and
`periods`.
Took the first of your two options: the five are out of
`SHARED_TIME_DIRECTIVES`, so a format using one leaves the column raw, the
same
answer `local!` and the composite directives get. Reasons for not applying
the
configured names instead:
The worker can read `D3_TIME_FORMAT`, but only the operator's overrides. The
names for every key left unset live in `DEFAULT_D3_TIME_FORMAT` on the
frontend,
so applying them means a second copy of those five lists in Python, and a
report
that goes quietly wrong if the two ever drift. That is the same kind of
mirror
this file already is, and a locale table is a worse thing to mirror than a
format string.
Leaving the column raw is also what the report did before this PR, so no
column
gets worse, and none of the eight presets in `D3_TIME_FORMAT_OPTIONS` uses a
name directive, so the loss is limited to a hand-typed format.
`test_table_skips_a_time_format_naming_a_month_weekday_or_period` pins all
five.
On the previous commit each one fails with the English name: `January 2024`,
`Jan 15`, `Monday`, `Mon`, `02:30 PM`.
Your comment also made me stop trusting how I built that set in the first
place, so I compared the two implementations directive by directive instead
of
by name, running `utcFormat('%<c>')` and `Timestamp.strftime('%<c>')` over
the
same instants. That turned up one more wrong entry, `%f`:
```
2024-01-15 14:30:00.123456 d3 "123000" strftime "123456"
```
d3 builds microseconds as `getUTCMilliseconds() * 1000`, so the browser's
value
always ends in `000`, while a warehouse column with sub-millisecond precision
reaches `strftime` with the real digits. `%f` is out of the set too, and
`%H:%M:%S.%f` joins the parametrized skip test.
The same comparison is also why I did not catch your finding myself: on this
worker, English and with nothing configured, all five name directives match
`strftime` exactly. The mismatch only exists against a configuration, which
is
the part I was not treating as an input.
One adjacent thing I found while checking this, not in this PR's scope but
you
should know it is there: `superset/utils/number_format.py` has the same
exposure
to `D3_FORMAT` and does not read it at all. With
`D3_FORMAT = {"decimal": ",", "thousands": "."}` the chart formats `1234.5`
as
`1.234,50` under `,.2f` and the report formats it as `1,234.50`. Setting it
changes nothing server-side. Happy to open that separately if you want it
tracked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
