You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2024/08/08 23:50:05 UTC
[PR] fix: parse pandas pivot null values [superset]
eschutho opened a new pull request, #29898:
URL: https://github.com/apache/superset/pull/29898
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
If a result has a null value for a pivot table with totals, we are seeing this error:
```
pandas._libs.lib.maybe_convert_numeric
ValueError: Unable to parse string "NULL"
```
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769254370
##########
superset/charts/post_processing.py:
##########
@@ -151,6 +153,18 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
# add subtotal for each group and overall total; we start from the
# overall group, and iterate deeper into subgroups
groups = df.columns
+ if not apply_metrics_on_rows:
+ for col in df.columns:
+ # we need to replace the temporary placeholder with either a string
+ # or np.nan, depending on the column type so that they can sum correctly
+ if pd.api.types.is_numeric_dtype(df[col]):
+ df[col].replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
+ else:
+ df[col].replace("SUPERSET_PANDAS_NAN", "nan", inplace=True)
Review Comment:
I chose the string "nan" here because that is the default behavior when there is a null value when pivoting without sums.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1775416916
##########
superset/charts/post_processing.py:
##########
@@ -86,7 +87,8 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
# pivot data; we'll compute totals and subtotals later
if rows or columns:
# pivoting with null values will create an empty df
- df = df.fillna("NULL")
Review Comment:
@mistercrunch the problem here is that we do the pivot in Pandas (for reports and CSV download), and it will fail if the dataframe has `NaN`s.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1766214170
##########
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:
Hi, my name is NULL, and my last name is '); DROP TABLE users; --
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "andy-clapson (via GitHub)" <gi...@apache.org>.
andy-clapson commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1719894347
##########
tests/unit_tests/charts/test_post_processing.py:
##########
Review Comment:
test coverage for nulls here was well-needed! great
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1766215728
##########
superset/charts/post_processing.py:
##########
@@ -86,7 +87,8 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
# pivot data; we'll compute totals and subtotals later
if rows or columns:
# pivoting with null values will create an empty df
- df = df.fillna("NULL")
Review Comment:
mmmh, seems the frontend should be doing this ...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769253071
##########
tests/unit_tests/charts/test_post_processing.py:
##########
@@ -407,6 +407,346 @@ def test_pivot_df_single_row_two_metrics():
)
+def test_pivot_df_single_row_null_values():
+ """
+ Pivot table when a single column and 2 metrics are selected.
+ """
+ df = pd.DataFrame.from_dict(
+ {
+ "gender": {0: "girl", 1: "boy"},
+ "SUM(num)": {0: 118065, 1: None},
+ "MAX(num)": {0: 2588, 1: None},
+ }
+ )
+ assert (
+ df.to_markdown()
+ == """
+| | gender | SUM(num) | MAX(num) |
+|---:|:---------|-----------:|-----------:|
+| 0 | girl | 118065 | 2588 |
+| 1 | boy | nan | nan |
+ """.strip()
+ )
+
+ pivoted = pivot_df(
+ df,
+ rows=["gender"],
+ columns=[],
+ metrics=["SUM(num)", "MAX(num)"],
+ aggfunc="Sum",
+ transpose_pivot=False,
+ combine_metrics=False,
+ show_rows_total=False,
Review Comment:
yah, I don't think it would fail in this case, but I added a test for all the combos.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "andy-clapson (via GitHub)" <gi...@apache.org>.
andy-clapson commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769195922
##########
superset/charts/post_processing.py:
##########
@@ -171,7 +174,7 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
for subgroup in subgroups:
slice_ = df.index.get_loc(subgroup)
subtotal = pivot_v2_aggfunc_map[aggfunc](
- df.iloc[slice_, :].apply(pd.to_numeric), axis=0
+ df.iloc[slice_, :].apply(pd.to_numeric, errors="coerce"), axis=0
Review Comment:
it's `loc`s and `iloc`s all the way down 🐢
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769247280
##########
superset/charts/post_processing.py:
##########
@@ -186,6 +200,14 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
if apply_metrics_on_rows:
df = df.T
+ # replace the remaining temporary placeholder string for np.nan after pivoting
+ df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
+ df.rename(
+ index={"SUPERSET_PANDAS_NAN": np.nan},
+ columns={"SUPERSET_PANDAS_NAN": np.nan},
+ inplace=True,
+ )
+
Review Comment:
Converting the values back so that we don't print "SUPERSET_PANDAS_NAN"
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769254370
##########
superset/charts/post_processing.py:
##########
@@ -151,6 +153,18 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
# add subtotal for each group and overall total; we start from the
# overall group, and iterate deeper into subgroups
groups = df.columns
+ if not apply_metrics_on_rows:
+ for col in df.columns:
+ # we need to replace the temporary placeholder with either a string
+ # or np.nan, depending on the column type so that they can sum correctly
+ if pd.api.types.is_numeric_dtype(df[col]):
+ df[col].replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
+ else:
+ df[col].replace("SUPERSET_PANDAS_NAN", "nan", inplace=True)
Review Comment:
I chose the string "nan" here because that is the default behavior when there is a null value, without pivoting.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1766214170
##########
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:
Hi, my name is NULL, and my last name is `'); DROP TABLE users; --`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769247070
##########
superset/charts/post_processing.py:
##########
@@ -151,6 +153,18 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
# add subtotal for each group and overall total; we start from the
# overall group, and iterate deeper into subgroups
groups = df.columns
+ if not apply_metrics_on_rows:
+ for col in df.columns:
+ # we need to replace the temporary placeholder with either a string
+ # or np.nan, depending on the column type so that they can sum correctly
+ if pd.api.types.is_numeric_dtype(df[col]):
+ df[col].replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
+ else:
+ df[col].replace("SUPERSET_PANDAS_NAN", "nan", inplace=True)
+ else:
+ # when we applied metrics on rows, we switched the columns and rows
+ # so checking column type doesn't apply. Replace everything with np.nan
+ df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
Review Comment:
@betodealmeida we need this section here when totaling so that we 1) can sum with numbers (by converting the string "SUPERSET_PANDAS_NAN" with np.nan or 2) can sum with a string value. I'm using "nan" so that we don't print "SUPERSET_PANDAS_NAN".
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "andy-clapson (via GitHub)" <gi...@apache.org>.
andy-clapson commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1719892804
##########
superset/charts/post_processing.py:
##########
@@ -171,7 +174,7 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
for subgroup in subgroups:
slice_ = df.index.get_loc(subgroup)
subtotal = pivot_v2_aggfunc_map[aggfunc](
- df.iloc[slice_, :].apply(pd.to_numeric), axis=0
+ df.iloc[slice_, :].apply(pd.to_numeric, errors="coerce"), axis=0
Review Comment:
i think this might be the only change needed to deal with this issue: https://github.com/apache/superset/issues/27499
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1774243678
##########
superset/charts/post_processing.py:
##########
@@ -86,7 +87,8 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
# pivot data; we'll compute totals and subtotals later
if rows or columns:
# pivoting with null values will create an empty df
- df = df.fillna("NULL")
Review Comment:
@mistercrunch I'm not sure I got this..
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769251499
##########
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:
@betodealmeida and I met and talked about using a different placeholder string that we thought would be an unlikely "real" value: `SUPERSET_PANDAS_NAN`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "andy-clapson (via GitHub)" <gi...@apache.org>.
andy-clapson commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1719895205
##########
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:
lol, Bobby Tables.
Are you suggesting this chart has a user-defined option to fill nulls with <something>? or maybe a drop-down of some value?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769190158
##########
superset/charts/post_processing.py:
##########
@@ -171,7 +174,7 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
for subgroup in subgroups:
slice_ = df.index.get_loc(subgroup)
subtotal = pivot_v2_aggfunc_map[aggfunc](
- df.iloc[slice_, :].apply(pd.to_numeric), axis=0
+ df.iloc[slice_, :].apply(pd.to_numeric, errors="coerce"), axis=0
Review Comment:
pandas is so opaque, especially when you haven't touched it for years - `.iloc[]?`, "coerce"!?!? might be worth adding a comment that explains what it's doing
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "andy-clapson (via GitHub)" <gi...@apache.org>.
andy-clapson commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1719893542
##########
tests/unit_tests/charts/test_post_processing.py:
##########
@@ -407,6 +407,346 @@ def test_pivot_df_single_row_two_metrics():
)
+def test_pivot_df_single_row_null_values():
+ """
+ Pivot table when a single column and 2 metrics are selected.
+ """
+ df = pd.DataFrame.from_dict(
+ {
+ "gender": {0: "girl", 1: "boy"},
+ "SUM(num)": {0: 118065, 1: None},
+ "MAX(num)": {0: 2588, 1: None},
+ }
+ )
+ assert (
+ df.to_markdown()
+ == """
+| | gender | SUM(num) | MAX(num) |
+|---:|:---------|-----------:|-----------:|
+| 0 | girl | 118065 | 2588 |
+| 1 | boy | nan | nan |
+ """.strip()
+ )
+
+ pivoted = pivot_df(
+ df,
+ rows=["gender"],
+ columns=[],
+ metrics=["SUM(num)", "MAX(num)"],
+ aggfunc="Sum",
+ transpose_pivot=False,
+ combine_metrics=False,
+ show_rows_total=False,
Review Comment:
weirdly, the case I have where i replicate this NULL issue only occurs when I have one of the column or row total set to True.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #29898:
URL: https://github.com/apache/superset/pull/29898#discussion_r1769247070
##########
superset/charts/post_processing.py:
##########
@@ -151,6 +153,18 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s
# add subtotal for each group and overall total; we start from the
# overall group, and iterate deeper into subgroups
groups = df.columns
+ if not apply_metrics_on_rows:
+ for col in df.columns:
+ # we need to replace the temporary placeholder with either a string
+ # or np.nan, depending on the column type so that they can sum correctly
+ if pd.api.types.is_numeric_dtype(df[col]):
+ df[col].replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
+ else:
+ df[col].replace("SUPERSET_PANDAS_NAN", "nan", inplace=True)
+ else:
+ # when we applied metrics on rows, we switched the columns and rows
+ # so checking column type doesn't apply. Replace everything with np.nan
+ df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True)
Review Comment:
@betodealmeida we need this section here (from line 156) when totaling so that we 1) can sum with numbers (by converting the string "SUPERSET_PANDAS_NAN" with np.nan or 2) can sum with a string value. I'm using "nan" so that we don't print "SUPERSET_PANDAS_NAN".
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: parse pandas pivot null values [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho merged PR #29898:
URL: https://github.com/apache/superset/pull/29898
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org