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 2020/05/13 21:56:24 UTC
[GitHub] [incubator-superset] john-bodley opened a new pull request #9800: [mypy] Enforcing typing for translations
john-bodley opened a new pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800
### SUMMARY
Adding `mypy` type enforcement to the `superset.translations` module.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
CI.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### Reviewers
to: @etr2460 @villebro
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#issuecomment-628268977
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=h1) Report
> Merging [#9800](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cf30e1655081ef886c0d5f0484898bd5e0142beb&el=desc) will **increase** coverage by `6.29%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9800 +/- ##
==========================================
+ Coverage 64.35% 70.64% +6.29%
==========================================
Files 536 588 +52
Lines 29106 30472 +1366
Branches 2806 3153 +347
==========================================
+ Hits 18732 21528 +2796
+ Misses 10194 8824 -1370
+ Partials 180 120 -60
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `52.84% <ø> (-0.87%)` | :arrow_down: |
| #javascript | `59.02% <ø> (?)` | |
| #python | `71.06% <100.00%> (ø)` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/translations/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdHJhbnNsYXRpb25zL3V0aWxzLnB5) | `87.50% <100.00%> (ø)` | |
| [superset-frontend/src/setup/setupPlugins.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2lucy50cw==) | `8.00% <0.00%> (-92.00%)` | :arrow_down: |
| [...rset-frontend/src/setup/setupErrorMessagesExtra.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlc0V4dHJhLnRz) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
| [superset-frontend/src/setup/setupApp.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQXBwLnRz) | `25.00% <0.00%> (-3.58%)` | :arrow_down: |
| [...end/src/SqlLab/components/TemplateParamsEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RlbXBsYXRlUGFyYW1zRWRpdG9yLmpzeA==) | `88.57% <0.00%> (-2.86%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIuanN4) | `44.00% <0.00%> (-2.00%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <0.00%> (ø)` | |
| [superset-frontend/src/SqlLab/utils/sqlKeywords.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi91dGlscy9zcWxLZXl3b3Jkcy50cw==) | `100.00% <0.00%> (ø)` | |
| ... and [188 more](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=footer). Last update [cf30e16...7486f28](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#issuecomment-628268977
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=h1) Report
> Merging [#9800](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cf30e1655081ef886c0d5f0484898bd5e0142beb&el=desc) will **increase** coverage by `6.50%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9800 +/- ##
==========================================
+ Coverage 64.35% 70.85% +6.50%
==========================================
Files 536 588 +52
Lines 29106 30472 +1366
Branches 2806 3153 +347
==========================================
+ Hits 18732 21592 +2860
+ Misses 10194 8767 -1427
+ Partials 180 113 -67
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `53.65% <ø> (-0.06%)` | :arrow_down: |
| #javascript | `59.02% <ø> (?)` | |
| #python | `71.06% <100.00%> (ø)` | |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/translations/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdHJhbnNsYXRpb25zL3V0aWxzLnB5) | `87.50% <100.00%> (ø)` | |
| [superset-frontend/src/setup/setupPlugins.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2lucy50cw==) | `8.00% <0.00%> (-92.00%)` | :arrow_down: |
| [...rset-frontend/src/setup/setupErrorMessagesExtra.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlc0V4dHJhLnRz) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [superset-frontend/src/setup/setupApp.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQXBwLnRz) | `25.00% <0.00%> (-3.58%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <0.00%> (ø)` | |
| [superset-frontend/src/SqlLab/utils/sqlKeywords.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi91dGlscy9zcWxLZXl3b3Jkcy50cw==) | `100.00% <0.00%> (ø)` | |
| [...et-frontend/src/dashboard/util/isDashboardEmpty.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2lzRGFzaGJvYXJkRW1wdHkudHM=) | `100.00% <0.00%> (ø)` | |
| [...rontend/src/SqlLab/components/QueryAutoRefresh.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2guanN4) | `65.90% <0.00%> (ø)` | |
| [...rontend/src/messageToasts/enhancers/withToasts.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL21lc3NhZ2VUb2FzdHMvZW5oYW5jZXJzL3dpdGhUb2FzdHMudHN4) | `100.00% <0.00%> (ø)` | |
| ... and [186 more](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=footer). Last update [cf30e16...7486f28](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] john-bodley commented on a change in pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#discussion_r425613370
##########
File path: superset/translations/utils.py
##########
@@ -38,7 +38,7 @@ def get_language_pack(locale):
try:
with open(filename, encoding="utf8") as f:
pack = json.load(f)
- ALL_LANGUAGE_PACKS[locale] = pack
+ ALL_LANGUAGE_PACKS[locale] = pack or {}
except Exception: # pylint: disable=broad-except
# Assuming english, client side falls back on english
Review comment:
@etr2460 the idea here was to keep the status quo. `pack` could be `None` per line 35 and remain that way if an exception is thrown either when opening the file or parsing the JSON.
----------------------------------------------------------------
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.
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] [incubator-superset] villebro commented on a change in pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#discussion_r424867975
##########
File path: superset/translations/utils.py
##########
@@ -16,15 +16,15 @@
# under the License.
import json
import os
-from typing import Any, Dict
+from typing import Any, Dict, Optional
# Global caching for JSON language packs
ALL_LANGUAGE_PACKS: Dict[str, Dict[Any, Any]] = {"en": {}}
DIR = os.path.dirname(os.path.abspath(__file__))
-def get_language_pack(locale):
+def get_language_pack(locale: str) -> Optional[Dict[Any, Any]]:
Review comment:
I believe we could make the key `str` to improve differentiation slightly without complicating things too much (strictly speaking it seems the structure is `Optional[Dict[str, Union[str, Dict[str, Union[Dict[str, str], List[str]]]]]]` which quite frankly looks awful and probably won't help detect any errors)
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io commented on pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#issuecomment-628268977
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=h1) Report
> Merging [#9800](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cf30e1655081ef886c0d5f0484898bd5e0142beb&el=desc) will **increase** coverage by `6.47%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9800 +/- ##
==========================================
+ Coverage 64.35% 70.83% +6.47%
==========================================
Files 536 184 -352
Lines 29106 17861 -11245
Branches 2806 0 -2806
==========================================
- Hits 18732 12652 -6080
+ Misses 10194 5209 -4985
+ Partials 180 0 -180
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #python | `70.83% <100.00%> (-0.23%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/translations/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdHJhbnNsYXRpb25zL3V0aWxzLnB5) | `87.50% <100.00%> (ø)` | |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `58.92% <0.00%> (-21.43%)` | :arrow_down: |
| [superset/utils/cache.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2FjaGUucHk=) | `45.83% <0.00%> (-20.84%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `78.26% <0.00%> (-13.05%)` | :arrow_down: |
| [superset/views/database/api.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvYXBpLnB5) | `83.90% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.29% <0.00%> (-2.71%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `83.77% <0.00%> (-2.36%)` | :arrow_down: |
| [superset/jinja\_context.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvamluamFfY29udGV4dC5weQ==) | `84.69% <0.00%> (-1.03%)` | :arrow_down: |
| [superset/security/manager.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `88.77% <0.00%> (-0.36%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.88% <0.00%> (-0.23%)` | :arrow_down: |
| ... and [353 more](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=footer). Last update [cf30e16...7486f28](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] etr2460 commented on a change in pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#discussion_r425251345
##########
File path: superset/translations/utils.py
##########
@@ -38,7 +38,7 @@ def get_language_pack(locale):
try:
with open(filename, encoding="utf8") as f:
pack = json.load(f)
- ALL_LANGUAGE_PACKS[locale] = pack
+ ALL_LANGUAGE_PACKS[locale] = pack or {}
except Exception: # pylint: disable=broad-except
# Assuming english, client side falls back on english
Review comment:
Perhaps we should put:
```python
pack = {}
```
in this block, and move
```python
ALL_LANGUAGE_PACKS[locale] = pack
```
outside of the try/except.
Then we could remove the `Optional` from the `get_language_pack` return type
----------------------------------------------------------------
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.
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] [incubator-superset] etr2460 commented on a change in pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#discussion_r425598471
##########
File path: superset/translations/utils.py
##########
@@ -38,7 +38,7 @@ def get_language_pack(locale):
try:
with open(filename, encoding="utf8") as f:
pack = json.load(f)
- ALL_LANGUAGE_PACKS[locale] = pack
+ ALL_LANGUAGE_PACKS[locale] = pack or {}
except Exception: # pylint: disable=broad-except
# Assuming english, client side falls back on english
Review comment:
Doesn't an empty dict resolve to false? so if the locale is English then this returns an empty dict (based on the constant at the top of the file)
----------------------------------------------------------------
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.
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] [incubator-superset] john-bodley commented on a change in pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#discussion_r425323726
##########
File path: superset/translations/utils.py
##########
@@ -38,7 +38,7 @@ def get_language_pack(locale):
try:
with open(filename, encoding="utf8") as f:
pack = json.load(f)
- ALL_LANGUAGE_PACKS[locale] = pack
+ ALL_LANGUAGE_PACKS[locale] = pack or {}
except Exception: # pylint: disable=broad-except
# Assuming english, client side falls back on english
Review comment:
@etr2460 I thought of that however it would change the return type, i.e., it could no longer be `None` and per the comment it seems like this could be problematic for the client.
----------------------------------------------------------------
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.
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] [incubator-superset] codecov-io edited a comment on pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#issuecomment-628268977
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=h1) Report
> Merging [#9800](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/cf30e1655081ef886c0d5f0484898bd5e0142beb&el=desc) will **increase** coverage by `1.59%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9800 +/- ##
==========================================
+ Coverage 64.35% 65.94% +1.59%
==========================================
Files 536 588 +52
Lines 29106 30472 +1366
Branches 2806 3153 +347
==========================================
+ Hits 18732 20096 +1364
+ Misses 10194 10193 -1
- Partials 180 183 +3
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `59.02% <ø> (?)` | |
| #python | `70.83% <100.00%> (-0.23%)` | :arrow_down: |
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/translations/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdHJhbnNsYXRpb25zL3V0aWxzLnB5) | `87.50% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupPlugins.ts](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2lucy50cw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [283 more](https://codecov.io/gh/apache/incubator-superset/pull/9800/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=footer). Last update [cf30e16...7486f28](https://codecov.io/gh/apache/incubator-superset/pull/9800?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-superset] etr2460 commented on a change in pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#discussion_r425599063
##########
File path: superset/translations/utils.py
##########
@@ -38,7 +38,7 @@ def get_language_pack(locale):
try:
with open(filename, encoding="utf8") as f:
pack = json.load(f)
- ALL_LANGUAGE_PACKS[locale] = pack
+ ALL_LANGUAGE_PACKS[locale] = pack or {}
except Exception: # pylint: disable=broad-except
# Assuming english, client side falls back on english
Review comment:
And this seems to indicate that if this isn't set, we default to an empty object on the frontend anyway: https://github.com/apache-superset/superset-ui/blob/master/packages/superset-ui-translation/src/Translator.ts#L26
----------------------------------------------------------------
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.
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] [incubator-superset] etr2460 commented on a change in pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800#discussion_r426217424
##########
File path: superset/translations/utils.py
##########
@@ -38,7 +38,7 @@ def get_language_pack(locale):
try:
with open(filename, encoding="utf8") as f:
pack = json.load(f)
- ALL_LANGUAGE_PACKS[locale] = pack
+ ALL_LANGUAGE_PACKS[locale] = pack or {}
except Exception: # pylint: disable=broad-except
# Assuming english, client side falls back on english
Review comment:
I'm still not 100% convinced that it's not worth cleaning up a bit and making this not `Optional`, but if you don't want to make such a change in the typing PR, that's fine. I can make a follow up PR to update this logic as I think it's cleaner
----------------------------------------------------------------
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.
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] [incubator-superset] john-bodley merged pull request #9800: style: enforcing mypy typing for translations
Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #9800:
URL: https://github.com/apache/incubator-superset/pull/9800
----------------------------------------------------------------
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.
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