SEPURI-SAI-KRISHNA commented on code in PR #45410:
URL: https://github.com/apache/superset/pull/45410#discussion_r4385017259
##########
superset/charts/client_processing.py:
##########
@@ -835,6 +863,51 @@ def union_currency_context(
}
+def is_strftime_expressible(d3_time_format: str) -> bool:
+ """Whether ``strftime`` renders a d3 time format the way the browser
does."""
+ if LOCAL_TIME_PREFIX in d3_time_format:
+ # ``TimeFormatterRegistry.get`` strips the marker before building the
+ # formatter, so leaving it in would render it as literal text.
+ return False
+ directives = re.findall(r"%(.)", d3_time_format)
+ if not directives:
+ # Nothing for ``strftime`` to substitute, so it hands the string back
+ # unchanged and every cell would read the format itself. This is also
+ # what rejects the aliases the time format controls offer, which the
+ # browser resolves to a formatter that picks a representation per value
+ # rather than to a format string: ``smart_date`` ("Adaptive
+ # formatting", ``SMART_DATE_ID`` in ``superset-ui-core/time-format``)
+ # among them.
+ return False
+ if "%" in re.sub(r"%.", "", d3_time_format):
+ # A ``%`` no directive consumed, as the trailing one in ``%Y%``. d3
+ # drops it (``2021``) and ``strftime`` copies it (``2021%``), so the
+ # two disagree even where every directive is shared. ``%%`` is not
+ # this case: both read it as one escaped ``%``.
+ return False
+ return all(directive in SHARED_TIME_DIRECTIVES for directive in directives)
+
+
+def format_time_column(df: pd.DataFrame, column: Any, d3_time_format: str) ->
None:
+ """Format a temporal column in place with an explicit d3 time format.
+
+ The caller gates this on ``is_strftime_expressible``, so every directive in
+ ``d3_time_format`` means the same thing here as it does in the browser.
Null
+ values are left as-is rather than rendered as the string ``"NaT"``.
Review Comment:
You are right, and the behaviour was never the problem.
`pd.NaT.strftime("%Y")`
raises `ValueError: NaTType does not support strftime`, so without the
`pd.notna` guard one null fails the whole report rather than printing
`"NaT"`.
The guard itself is already pinned: dropping it fails both
`test_table_time_format_keeps_nulls` and
`test_table_report_time_format_survives_a_null_in_the_column`. So this was
the
wording alone, and the docstring and the test docstring now say what
`strftime`
does with a `NaT`.
--
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]
