SEPURI-SAI-KRISHNA commented on code in PR #45410:
URL: https://github.com/apache/superset/pull/45410#discussion_r4403758587
##########
superset/charts/client_processing.py:
##########
@@ -1568,6 +1676,44 @@ def _read_excel_for_client_processing(
return df.set_index(first_column)
+def payload_coltypes(
+ df: pd.DataFrame,
+ datasource: Optional[Union["BaseDatasource", "Query"]],
+) -> list[GenericDataType]:
+ """The payload's column types, reporting a text-rendered column as text.
+
+ ``extract_dataframe_dtypes`` lets the dataset's ``is_dttm`` flag decide the
+ type, so a temporal column a post-processor has rendered as text is still
+ reported as ``TEMPORAL``. ``chart_data_to_dataframe`` casts every
+ ``TEMPORAL`` column back to ``datetime64[ms]`` to build the notification
+ table, which would re-parse those strings and lose the configured format,
+ and would read ``02/01/2024`` month-first while reading ``15/01/2024``
+ day-first.
+
+ Only a column whose values really are strings is reported differently. The
+ XLSX path hands these types to ``apply_column_types`` to get date cells out
+ of a frame whose dates are still dates, and that frame is untouched here.
+
+ Nulls are dropped before the values are read. ``format_time_column`` leaves
+ them in place, and ``infer_dtype`` counts a ``NaT`` as a value of its own
+ type rather than one ``skipna`` drops, so it calls a formatted column
+ holding one ``"mixed"``. A column of nothing but nulls reads as neither
+ text nor mixed and keeps whatever type the dataset gives it.
+
+ The index is positional because ``extract_dataframe_dtypes`` walks the
+ frame the same way, so the two line up even where a label repeats.
+ """
+ coltypes = extract_dataframe_dtypes(df, datasource)
+ return [
+ GenericDataType.STRING
+ if coltype == GenericDataType.TEMPORAL
+ and pd.api.types.infer_dtype(df.iloc[:, index].dropna(), skipna=False)
Review Comment:
Confirmed. `in ("string", "empty")` keeps all 315 green, and `infer_dtype` on
the dropped series returns `"empty"` for an all-null column against
`"string"`
for the mixed one, which is why the existing null test passes either way.
Added `test_payload_coltypes_keeps_an_all_null_column_temporal`: an
all-`None`
object column flagged `is_dttm`, asserting `[GenericDataType.TEMPORAL]`. It
is
the only failure under that widening.
--
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]
