betodealmeida commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1710462821
##########
superset/charts/post_processing.py:
##########
@@ -150,6 +151,8 @@ def pivot_df( # pylint: disable=too-many-locals,
too-many-arguments, too-many-s
if show_rows_total:
# add subtotal for each group and overall total; we start from the
# overall group, and iterate deeper into subgroups
+ # Ensure "NULL" strings are replaced with NaN
+ df.replace("NULL", np.nan, inplace=True)
Review Comment:
I don't think we should do this automatically, ideally there should be an
option when building the pivot table to have this conversion. Or people could
do it as a derived column or virtual dataset.
Imagine I have a table of users and someone has the username 'NULL'. I don't
think we should do this conversion in that case. This is not hypothetical,
Instagram's data infra once went down because someone created a user called
`null`.
(Not as bad as when an employee broke the Facebook intranet by using their
initials as their username — www.)
##########
superset/charts/post_processing.py:
##########
@@ -150,6 +151,8 @@ def pivot_df( # pylint: disable=too-many-locals,
too-many-arguments, too-many-s
if show_rows_total:
# add subtotal for each group and overall total; we start from the
# overall group, and iterate deeper into subgroups
+ # Ensure "NULL" strings are replaced with NaN
+ df.replace("NULL", np.nan, inplace=True)
Review Comment:
Speaking of breaking things, looks like the GitHub auto-linker is broken...
--
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]
