Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
rusackas commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2557732229 Hi! Just coming back noticing that there's a conflict to resolve and some linting that needs to be done (you can use the pre-commit hook to do so). I've pinged @villebro for a review since he knows the post-processing bits better than anyone, and @kgabryje since he has a general pivot table situational awareness, but I suspect that we won't see this merged quite yet as people are now heading off for holiday breaks. We'll check back in as soon as we can. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
frlm commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2532191489 > thanks for formatting... kicking CI to run it again. Thank you for your patience 💯 -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
rusackas commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2532047492 thanks for formatting... kicking CI to run it agian. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
rusackas commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2523898521 Running CI again... then hopefully we can get a test environment spun up. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
frlm commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2514727749 Hi @kgabryje, can you review my pull request? Pre commit checks give me an failure (attached failure details) but my changes work. I cannot understand how to fix this issue... mypy.Failed - hook id: mypy - exit code: 1 superset/charts/post_processing.py:29: error: Name "app" already defined (by an import) [no-redef] superset/charts/post_processing.py:48: error: Module has no attribute "config" [attr-defined] -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
frlm commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2507463975 > I was trying to help with fixing (not fishing, as I mis-typed) some of the lining issues... hope that helps. Thanks @rusackas, what must I do now to get merge? -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
rusackas commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2499046519 I was trying to help with fixing (not fishing, as I mis-typed) some of the lining issues... hope that helps. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
github-actions[bot] commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2499047048 @rusackas No ephemeral environment action detected. Please use '/testenv up' or '/testenv down'. [View workflow run](https://github.com/apache/superset/actions/runs/12018875084). -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
rusackas commented on code in PR #30961:
URL: https://github.com/apache/superset/pull/30961#discussion_r1857351350
##
superset/charts/post_processing.py:
##
@@ -339,29 +344,32 @@ def apply_post_process(
query["indexnames"] = list(processed_df.index)
query["coltypes"] = extract_dataframe_dtypes(processed_df, datasource)
query["rowcount"] = len(processed_df.index)
-
-# Flatten hierarchical columns/index since they are represented as
-# `Tuple[str]`. Otherwise encoding to JSON later will fail because
-# maps cannot have tuples as their keys in JSON.
-processed_df.columns = [
-" ".join(str(name) for name in column).strip()
-if isinstance(column, tuple)
-else column
-for column in processed_df.columns
-]
-processed_df.index = [
-" ".join(str(name) for name in index).strip()
-if isinstance(index, tuple)
-else index
-for index in processed_df.index
-]
-
+
if query["result_format"] == ChartDataResultFormat.JSON:
+# Flatten hierarchical columns/index since they are represented as
+# `Tuple[str]`. Otherwise encoding to JSON later will fail because
+# maps cannot have tuples as their keys in JSON.
+processed_df.columns = [
+" ".join(str(name) for name in column).strip()
+if isinstance(column, tuple)
+else column
+for column in processed_df.columns
+]
+processed_df.index = [
+" ".join(str(name) for name in index).strip()
+if isinstance(index, tuple)
+else index
+for index in processed_df.index
+]
query["data"] = processed_df.to_dict()
+
elif query["result_format"] == ChartDataResultFormat.CSV:
buf = StringIO()
-processed_df.to_csv(buf)
+processed_df.to_csv(buf,
+sep=csv_export_settings.get('sep', ','),
+encoding=csv_export_settings.get('encoding',
'utf-8'),
+decimal=csv_export_settings.get('decimal',
'.'))
buf.seek(0)
query["data"] = buf.getvalue()
-return result
+return result
Review Comment:
```suggestion
return result
```
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
rusackas commented on code in PR #30961:
URL: https://github.com/apache/superset/pull/30961#discussion_r1857351013
##
superset/charts/post_processing.py:
##
@@ -339,29 +344,32 @@ def apply_post_process(
query["indexnames"] = list(processed_df.index)
query["coltypes"] = extract_dataframe_dtypes(processed_df, datasource)
query["rowcount"] = len(processed_df.index)
-
-# Flatten hierarchical columns/index since they are represented as
-# `Tuple[str]`. Otherwise encoding to JSON later will fail because
-# maps cannot have tuples as their keys in JSON.
-processed_df.columns = [
-" ".join(str(name) for name in column).strip()
-if isinstance(column, tuple)
-else column
-for column in processed_df.columns
-]
-processed_df.index = [
-" ".join(str(name) for name in index).strip()
-if isinstance(index, tuple)
-else index
-for index in processed_df.index
-]
-
+
if query["result_format"] == ChartDataResultFormat.JSON:
+# Flatten hierarchical columns/index since they are represented as
+# `Tuple[str]`. Otherwise encoding to JSON later will fail because
+# maps cannot have tuples as their keys in JSON.
+processed_df.columns = [
+" ".join(str(name) for name in column).strip()
+if isinstance(column, tuple)
+else column
+for column in processed_df.columns
+]
+processed_df.index = [
+" ".join(str(name) for name in index).strip()
+if isinstance(index, tuple)
+else index
+for index in processed_df.index
+]
query["data"] = processed_df.to_dict()
+
elif query["result_format"] == ChartDataResultFormat.CSV:
buf = StringIO()
-processed_df.to_csv(buf)
+processed_df.to_csv(buf,
+sep=csv_export_settings.get('sep', ','),
+encoding=csv_export_settings.get('encoding',
'utf-8'),
+decimal=csv_export_settings.get('decimal',
'.'))
Review Comment:
```suggestion
processed_df.to_csv(buf,
sep=csv_export_settings.get('sep', ','),
encoding=csv_export_settings.get('encoding',
'utf-8'),
decimal=csv_export_settings.get('decimal',
'.'))
```
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
rusackas commented on code in PR #30961:
URL: https://github.com/apache/superset/pull/30961#discussion_r1857350101
##
superset/charts/post_processing.py:
##
@@ -327,8 +329,11 @@ def apply_post_process(
if query["result_format"] == ChartDataResultFormat.JSON:
df = pd.DataFrame.from_dict(data)
elif query["result_format"] == ChartDataResultFormat.CSV:
-df = pd.read_csv(StringIO(data))
-
+df = pd.read_csv(StringIO(data),
+ sep=csv_export_settings.get('sep', ','),
+ encoding=csv_export_settings.get('encoding',
'utf-8'),
+ decimal=csv_export_settings.get('decimal', '.'))
+
Review Comment:
```suggestion
df = pd.read_csv(StringIO(data),
sep=csv_export_settings.get('sep', ','),
encoding=csv_export_settings.get('encoding',
'utf-8'),
decimal=csv_export_settings.get('decimal', '.'))
```
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
codecov[bot] commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2499026911 ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/30961?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 65.35%. Comparing base [(`76d897e`)](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`86521a4`)](https://app.codecov.io/gh/apache/superset/commit/86521a4de5e2550f3ed130852b6e8d8dc012310e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > Report is 1065 commits behind head on master. Additional details and impacted files ```diff @@Coverage Diff @@ ## master #30961 +/- ## == + Coverage 60.48% 65.35% +4.86% == Files1931 536-1395 Lines 7623638961 -37275 Branches 85680-8568 == - Hits4611425462 -20652 + Misses 2801713499 -14518 + Partials 21050-2105 ``` | [Flag](https://app.codecov.io/gh/apache/superset/pull/30961/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [hive](https://app.codecov.io/gh/apache/superset/pull/30961/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.82% <28.57%> (-0.34%)` | :arrow_down: | | [javascript](https://app.codecov.io/gh/apache/superset/pull/30961/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [presto](https://app.codecov.io/gh/apache/superset/pull/30961/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.30% <28.57%> (-0.50%)` | :arrow_down: | | [python](https://app.codecov.io/gh/apache/superset/pull/30961/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.35% <100.00%> (+1.86%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/apache/superset/pull/30961/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.83% <100.00%> (+3.20%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/30961?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] fix(csv_export): use custom CSV_EXPORT parameters in pd.read_csv for pivot table [superset]
frlm commented on PR #30961: URL: https://github.com/apache/superset/pull/30961#issuecomment-2490449351 I don't understand what I have to do, can you check if there are issues? -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
