You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/06 19:52:29 UTC
[GitHub] [superset] codemaster08240328 opened a new pull request, #19575: fix: Show full long number in text email report for table chart.
codemaster08240328 opened a new pull request, #19575:
URL: https://github.com/apache/superset/pull/19575
<!---
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 -->
Before this update, when we have long number in text email report for table chart, report was not showing correct value.
It was because `pd` converts long float value into `e` type.
In this pr, we added some display viz option into `pd`, so all long float value are displayed as a string.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
BEFORE:
<img width="458" alt="image" src="https://user-images.githubusercontent.com/39701522/162058491-558fe7fe-291c-46b6-a130-fad22a7e56dd.png">
AFTER:
<img width="373" alt="image" src="https://user-images.githubusercontent.com/39701522/162058433-5ef83edc-4d4f-4e8c-b0b7-09544e3487a6.png">
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. Create a virtual dataset using this sql:
```
select product_line, (sum(price_each)) * 12314 as Calculation
from "Vehicle Sales"
group by 1
```
2. Create a table chart using Virtual Dataset.
3. create a report sending this chart by email as text
Check the email.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Bug fix
--
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
[GitHub] [superset] codecov[bot] commented on pull request #19575: fix: Show full long number in text email report for table chart.
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19575:
URL: https://github.com/apache/superset/pull/19575#issuecomment-1093239893
# [Codecov](https://codecov.io/gh/apache/superset/pull/19575?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19575](https://codecov.io/gh/apache/superset/pull/19575?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3c01957) into [master](https://codecov.io/gh/apache/superset/commit/9a9e3b6e3bafd9a0d58b2f49404c19b31d2bc48a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9a9e3b6) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 3c01957 differs from pull request most recent head 6d882c2. Consider uploading reports for the commit 6d882c2 to get more accurate results
```diff
@@ Coverage Diff @@
## master #19575 +/- ##
==========================================
- Coverage 66.49% 66.46% -0.03%
==========================================
Files 1681 1681
Lines 64370 64371 +1
Branches 6583 6583
==========================================
- Hits 42803 42786 -17
- Misses 19886 19904 +18
Partials 1681 1681
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.66% <0.00%> (-0.01%)` | :arrow_down: |
| mysql | `?` | |
| postgres | `81.93% <100.00%> (-0.02%)` | :arrow_down: |
| presto | `52.51% <0.00%> (-0.01%)` | :arrow_down: |
| python | `82.32% <100.00%> (-0.06%)` | :arrow_down: |
| sqlite | `81.71% <100.00%> (+<0.01%)` | :arrow_up: |
| unit | `?` | |
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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/19575?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/utils/csv.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY3N2LnB5) | `97.67% <100.00%> (+0.05%)` | :arrow_up: |
| [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `93.97% <0.00%> (-3.62%)` | :arrow_down: |
| [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `92.20% <0.00%> (-1.30%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.36% <0.00%> (-0.72%)` | :arrow_down: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `94.73% <0.00%> (-0.53%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `76.55% <0.00%> (-0.46%)` | :arrow_down: |
| [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `91.14% <0.00%> (-0.37%)` | :arrow_down: |
| [superset/initialization/\_\_init\_\_.py](https://codecov.io/gh/apache/superset/pull/19575/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvaW5pdGlhbGl6YXRpb24vX19pbml0X18ucHk=) | `91.31% <0.00%> (-0.35%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19575?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19575?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9a9e3b6...6d882c2](https://codecov.io/gh/apache/superset/pull/19575?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [superset] rusackas merged pull request #19575: fix: Show full long number in text email report for table chart.
Posted by GitBox <gi...@apache.org>.
rusackas merged PR #19575:
URL: https://github.com/apache/superset/pull/19575
--
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
[GitHub] [superset] rusackas commented on pull request #19575: fix: Show full long number in text email report for table chart.
Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #19575:
URL: https://github.com/apache/superset/pull/19575#issuecomment-1098294418
Approving (and re-starting CI... sigh). Heads up to @eschutho in case you have any additional feelings here.
--
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
[GitHub] [superset] diegomedina248 commented on a diff in pull request #19575: fix: Show full long number in text email report for table chart.
Posted by GitBox <gi...@apache.org>.
diegomedina248 commented on code in PR #19575:
URL: https://github.com/apache/superset/pull/19575#discussion_r844342287
##########
superset/utils/csv.py:
##########
@@ -95,6 +98,9 @@ def get_chart_dataframe(
return None
result = simplejson.loads(content.decode("utf-8"))
+ pd.set_option(
+ "display.float_format", lambda x: str(x)
+ ) # need to convert float value to string to show full long number
Review Comment:
add the comment above the line:
```
# comm
pd.set_option...
```
##########
superset/utils/csv.py:
##########
@@ -22,6 +23,8 @@
import pandas as pd
import simplejson
+logger = logging.getLogger(__name__)
Review Comment:
this and the import is probably not needed
--
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