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/05/16 09:17:47 UTC
[GitHub] [superset] ebaratte opened a new pull request, #20075: feat: custom d3 number locale
ebaratte opened a new pull request, #20075:
URL: https://github.com/apache/superset/pull/20075
### SUMMARY
This PR is an attempt to provide a way of customising the format of the numbers and currency displayed in superset frontend.
It is linked to (at least) issues #3972, #18938, #11328, #8913
Please also look at PR #17796, which provides a different approach to the same issue.
Design choice:
- the user can change the following d3 **number** formatting parameters: decimal, thousands, grouping, currency, numerals, percent, minus and nan
- this change does not include the **time formatting** parameters, since I believe it to be a different issue
- unlike #17796, it does not tie the format to the `LANGUAGES` config key: the user can use different parameters for the frontend language and number formatting
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before
After:
### TESTING INSTRUCTIONS
- in config.py, add a D3_FORMAT dictionary with [d3 locale settings](https://github.com/d3/d3-format/blob/main/README.md#locale_format)
- check the number formatting on any chart
### ADDITIONAL INFORMATION
- [x] Has associated issue: #3972, #18938, #11328, #8913
- [x] 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
- [x] 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
[GitHub] [superset] codecov[bot] commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1127457549
# [Codecov](https://codecov.io/gh/apache/superset/pull/20075?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 [#20075](https://codecov.io/gh/apache/superset/pull/20075?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a65ad27) into [master](https://codecov.io/gh/apache/superset/commit/4435e53901df4d64992a540694fbd3d5489c2220?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4435e53) will **decrease** coverage by `12.11%`.
> The diff coverage is `75.00%`.
```diff
@@ Coverage Diff @@
## master #20075 +/- ##
===========================================
- Coverage 66.37% 54.26% -12.12%
===========================================
Files 1715 1715
Lines 64179 64180 +1
Branches 6753 6753
===========================================
- Hits 42602 34826 -7776
- Misses 19859 27636 +7777
Partials 1718 1718
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.56% <100.00%> (+<0.01%)` | :arrow_up: |
| python | `57.40% <100.00%> (-25.14%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `49.14% <100.00%> (+<0.01%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/20075?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-frontend/src/preamble.ts](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ByZWFtYmxlLnRz) | `0.00% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupFormatters.ts](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy50cw==) | `0.00% <0.00%> (ø)` | |
| [superset/views/base.py](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQvdmlld3MvYmFzZS5weQ==) | `54.78% <ø> (-19.15%)` | :arrow_down: |
| [...perset-ui-chart-controls/src/utils/D3Formatting.ts](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3V0aWxzL0QzRm9ybWF0dGluZy50cw==) | `100.00% <100.00%> (ø)` | |
| [...-core/src/number-format/NumberFormatterRegistry.ts](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvbnVtYmVyLWZvcm1hdC9OdW1iZXJGb3JtYXR0ZXJSZWdpc3RyeS50cw==) | `100.00% <100.00%> (ø)` | |
| [.../number-format/NumberFormatterRegistrySingleton.ts](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvbnVtYmVyLWZvcm1hdC9OdW1iZXJGb3JtYXR0ZXJSZWdpc3RyeVNpbmdsZXRvbi50cw==) | `100.00% <100.00%> (ø)` | |
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQvY29uZmlnLnB5) | `91.02% <100.00%> (-0.31%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/key\_value/commands/upsert.py](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3Vwc2VydC5weQ==) | `0.00% <0.00%> (-89.59%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/20075/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-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-89.37%)` | :arrow_down: |
| ... and [279 more](https://codecov.io/gh/apache/superset/pull/20075/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/20075?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/20075?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 [4435e53...a65ad27](https://codecov.io/gh/apache/superset/pull/20075?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] MarkusBellmann commented on pull request #20075: feat: custom d3 number locale
Posted by "MarkusBellmann (via GitHub)" <gi...@apache.org>.
MarkusBellmann commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1505432417
Please, we really need this feature!
--
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] theYaro commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
theYaro commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1280855534
Waiting for this 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-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] ebaratte commented on pull request #20075: feat: custom d3 number locale
Posted by "ebaratte (via GitHub)" <gi...@apache.org>.
ebaratte commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1507111374
I rebased the PR, we should be good to go.
--
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] ebaratte commented on a diff in pull request #20075: feat: custom d3 number locale
Posted by "ebaratte (via GitHub)" <gi...@apache.org>.
ebaratte commented on code in PR #20075:
URL: https://github.com/apache/superset/pull/20075#discussion_r1176298504
##########
superset-frontend/src/types/bootstrapTypes.ts:
##########
@@ -150,6 +151,7 @@ export interface CommonBootstrapData {
extra_sequential_color_schemes: SequentialSchemeConfig[];
theme_overrides: JsonObject;
menu_data: MenuData;
+ d3_format: Partial<FormatLocaleDefinition>;
Review Comment:
I think we do; the way the code is set up, a user can specify a subset of the formatting options in `config.py` (`total=False` in the type definition).
The default values are set in the front (`D3FormatConfig.ts`, and `setD3Format()` in `NumberFormatterRegistry.ts:52`)
--
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] github-actions[bot] commented on pull request #20075: feat: custom d3 number locale
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526496926
@rusackas Ephemeral environment creation failed. Please check the Actions logs for details.
--
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] SergeRomankevich commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
SergeRomankevich commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1131197314
Tables with different currencies are crying aside and waiting for the 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-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] pm-at commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
pm-at commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1379901056
I'm sorry for repeated nudge, but this is a really helpful feature. Waiting for this PR to be merged..
--
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 #20075: feat: custom d3 number locale
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526495471
/testenv 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-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] github-actions[bot] commented on pull request #20075: feat: custom d3 number locale
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526536811
@rusackas Ephemeral environment creation failed. Please check the Actions logs for details.
--
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 #20075: feat: custom d3 number locale
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526562100
/testenv 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-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] cwegener commented on pull request #20075: feat: custom d3 number locale
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1532367884
Just cross-linking the alternative designs for future reference (somehow Github doesn't place links from Discussion threads for issue mentions into the issue log automatically ...)
https://github.com/apache/superset/discussions/23684
--
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] cwegener commented on pull request #20075: feat: custom d3 number locale
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1588176706
@phifa
According to the documentation, this config should be what is needed:
```
D3_FORMAT = {
"decimal": ".",
"thousands": ",",
"grouping": [3],
"currency": ["€", ""]
}
```
--
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] phifa commented on pull request #20075: feat: custom d3 number locale
Posted by "phifa (via GitHub)" <gi...@apache.org>.
phifa commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1587162953
So I am not familiar with `D3_FORMAT`. Can someone tell me what I need to add to `config.py`? I want this number to be displayed in a european format, like this: 579 781 or like this 579.781,000. Thanks a ton!
<img width="1337" alt="" src="https://github.com/apache/superset/assets/3061129/a38a3786-9704-44e2-b786-4bffce7c03ed">
--
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] pm-at commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
pm-at commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1174960986
Is there a way to speed up the review process?
--
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] enryls commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
enryls commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1166331774
Great feature! Hope to see it merged soon
--
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] villebro commented on a diff in pull request #20075: feat: custom d3 number locale
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #20075:
URL: https://github.com/apache/superset/pull/20075#discussion_r1173362701
##########
superset-frontend/src/types/bootstrapTypes.ts:
##########
@@ -150,6 +151,7 @@ export interface CommonBootstrapData {
extra_sequential_color_schemes: SequentialSchemeConfig[];
theme_overrides: JsonObject;
menu_data: MenuData;
+ d3_format: Partial<FormatLocaleDefinition>;
Review Comment:
Do we really need `Partial` here? It appears that the added `dict` in `config.py` defines all properties of `FormatLocaleDefinition`. If not, let's make sure the type in `config.py` reflects the partialness (this could be done by adding `total=False` to the Python class that's extending `TypedDict`).
--
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] k7daniel commented on pull request #20075: feat: custom d3 number locale
Posted by "k7daniel (via GitHub)" <gi...@apache.org>.
k7daniel commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1523167298
Yay! Can't wait to test this out!!
--
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] Pedrolrbr commented on pull request #20075: feat: custom d3 number locale
Posted by "Pedrolrbr (via GitHub)" <gi...@apache.org>.
Pedrolrbr commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1516409543
@justinpark @eschutho @kgabryje @villebro any updates?
--
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] ebaratte commented on pull request #20075: feat: custom d3 number locale
Posted by "ebaratte (via GitHub)" <gi...@apache.org>.
ebaratte commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1521543756
> I know that Superset is badly lacking in this space, but the main issue I have with this approach is the fact that it assumes the deployment as a whole speaks one single locale. As we start to prepare for making Superset more localizable (see e.g. a WIP PR of mine that adds timezone support to the chart data API: #23511), it would be nice if we could start making gradual progress towards having user or chart specific locales. Let's say we want to have one chart that shows euros, and another that shows US dollars. This would not be possible with this setting.
>
> Having said that, I think this approach is good to cover use cases where the entire deployment wants to "speak" only one specific locale, as long as we leave open the door for making this overridable per user/chart. And based on my review this doesn't paint us into any corner, so I think this looks good. Thoughts @ebaratte @Pedrolrbr @kgabryje @rusackas ?
This is indeed the general idea of this PR: set default values for the whole instance. This is especially relevant for the currency, since there is no proper way at the moment to change the default currency, and displaying an incorrect currency makes the charts incorrect - whereas number formatting is more of a cosmetic issue. Obviously, it assumes that all the data is in a single currency.
It shouldn't prevent local overrides, like:
- setting a specific currency on a chart (#22374)
- using a number format that depends on the current locale (#17796), or via user settings
--
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] ebaratte commented on a diff in pull request #20075: feat: custom d3 number locale
Posted by "ebaratte (via GitHub)" <gi...@apache.org>.
ebaratte commented on code in PR #20075:
URL: https://github.com/apache/superset/pull/20075#discussion_r1176292525
##########
superset/config.py:
##########
@@ -377,6 +377,18 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
# incomplete and not well maintained.
LANGUAGES = {}
+
+# Override the default d3 locale format
+# Default values are equivalent to
+# D3_FORMAT = {
+# "decimal": ".", # - decimal place string (e.g., ".").
+# "thousands": ",", # - group separator string (e.g., ",").
+# "grouping": [3], # - array of group sizes (e.g., [3]), cycled as needed.
+# "currency": ["$", ""] # - currency prefix/suffix strings (e.g., ["$", ""])
+# }
+# https://github.com/d3/d3-format/blob/main/README.md#formatLocale
+D3_FORMAT: Dict[str, Any] = {}
Review Comment:
Sure, I updated the type definition
--
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] github-actions[bot] commented on pull request #20075: feat: custom d3 number locale
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1531952779
Ephemeral environment shutdown and build artifacts deleted.
--
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] villebro commented on pull request #20075: feat: custom d3 number locale
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1506507894
@ebaratte would you be able to rebase this PR to fix the conflicts?
--
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 #20075: feat: custom d3 number locale
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #20075:
URL: https://github.com/apache/superset/pull/20075
--
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 #20075: feat: custom d3 number locale
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1531951794
Had a nice long chat with @mistercrunch about this approach and a couple other approaches to improving currency handling in Superset. At the end of the day, I think all three have their place. Expect some more proposals and/or PRs in the future. In the meantime, this looks ready to go. Thanks for the PR!!!! Hitting the merge button!
--
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] lf-floriandin commented on pull request #20075: feat: custom d3 number locale
Posted by "lf-floriandin (via GitHub)" <gi...@apache.org>.
lf-floriandin commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1506428792
Would also love if this get reviewed and merged!
--
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] villebro commented on a diff in pull request #20075: feat: custom d3 number locale
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #20075:
URL: https://github.com/apache/superset/pull/20075#discussion_r1173362701
##########
superset-frontend/src/types/bootstrapTypes.ts:
##########
@@ -150,6 +151,7 @@ export interface CommonBootstrapData {
extra_sequential_color_schemes: SequentialSchemeConfig[];
theme_overrides: JsonObject;
menu_data: MenuData;
+ d3_format: Partial<FormatLocaleDefinition>;
Review Comment:
Do we really need `Partial` here? It appears that the added `dict` in `config.py` defines all properties of `FormatLocaleDefinition`. If not, let's make sure the type in `config.py` reflects the partialness.
##########
superset/config.py:
##########
@@ -377,6 +377,18 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
# incomplete and not well maintained.
LANGUAGES = {}
+
+# Override the default d3 locale format
+# Default values are equivalent to
+# D3_FORMAT = {
+# "decimal": ".", # - decimal place string (e.g., ".").
+# "thousands": ",", # - group separator string (e.g., ",").
+# "grouping": [3], # - array of group sizes (e.g., [3]), cycled as needed.
+# "currency": ["$", ""] # - currency prefix/suffix strings (e.g., ["$", ""])
+# }
+# https://github.com/d3/d3-format/blob/main/README.md#formatLocale
+D3_FORMAT: Dict[str, Any] = {}
Review Comment:
nit: as `TypedDict` is now a default thing in our currently supported Python versions, I'd prefer to create a `D3Format(TypedDict)` for 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
[GitHub] [superset] rusackas commented on pull request #20075: feat: custom d3 number locale
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526563507
Sorry for all the noise on the thread, folks! I'm trying to troubleshoot the ephemeral environment builds, so it needs a little kicking ;)
--
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] k7daniel commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
k7daniel commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1129986435
This is very cool. Would be great to just set D3_FORMAT and pass that in to the docker image.
--
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] KarenWeis commented on pull request #20075: feat: custom d3 number locale
Posted by GitBox <gi...@apache.org>.
KarenWeis commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1209597823
Are there plans to merge or include in 3.0 release?
--
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] ebaratte commented on pull request #20075: feat: custom d3 number locale
Posted by "ebaratte (via GitHub)" <gi...@apache.org>.
ebaratte commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1522980218
Rebased to fix the tests (#23805)
--
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 #20075: feat: custom d3 number locale
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526091307
Hoping to test this on an ephemeral build, but then I can't wait to click the Merge button on 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
[GitHub] [superset] rusackas commented on pull request #20075: feat: custom d3 number locale
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526089908
/testenv 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-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] github-actions[bot] commented on pull request #20075: feat: custom d3 number locale
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526092279
@rusackas Ephemeral environment spinning up at http://34.220.220.185:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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] github-actions[bot] commented on pull request #20075: feat: custom d3 number locale
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526565507
@rusackas Ephemeral environment spinning up at http://54.200.4.245:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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] github-actions[bot] commented on pull request #20075: feat: custom d3 number locale
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1526522859
@rusackas Ephemeral environment spinning up at http://52.41.16.248:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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] villebro commented on pull request #20075: feat: custom d3 number locale
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1521885439
> This is indeed the general idea of this PR: set default values for the whole instance. This is especially relevant for the currency, since there is no proper way at the moment to change the default currency, and displaying an incorrect currency makes the charts incorrect - whereas number formatting is more of a cosmetic issue. Obviously, it assumes that all the data is in a single currency.
>
> It shouldn't prevent local overrides, like:
>
> * setting a specific currency on a chart ([feat(formatters): [WIP] Adding a few currency formatters #22374](https://github.com/apache/superset/pull/22374))
> * using a number format that depends on the current locale ([feat: Create D3NumberFormatters with active Locale #17796](https://github.com/apache/superset/pull/17796)), or via user settings
Ok you don't need to convince me more @ebaratte 🙂 Looks good to go to me, but there appears to be some frontend UTs that are failing. Please take a look so we can get this in 👍
--
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] cwegener commented on pull request #20075: feat: custom d3 number locale
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1596748648
> Does it work on version 2.1.0 ? Nothing changes for me.
No. This PR is not part of 2.1
This PR is only available in the master branch.
--
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] dab21rus commented on pull request #20075: feat: custom d3 number locale
Posted by "dab21rus (via GitHub)" <gi...@apache.org>.
dab21rus commented on PR #20075:
URL: https://github.com/apache/superset/pull/20075#issuecomment-1596729009
> @phifa According to the documentation, this config should be what is needed:
>
> ```
> D3_FORMAT = {
> "decimal": ".",
> "thousands": ",",
> "grouping": [3],
> "currency": ["€", ""]
> }
> ```
Does it work on version 2.1.0 ? Nothing changes for me.
--
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