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 2021/02/04 19:26:20 UTC
[GitHub] [superset] rwspielman opened a new pull request #12937: fix: chart keys in MultiLineViz
rwspielman opened a new pull request #12937:
URL: https://github.com/apache/superset/pull/12937
### SUMMARY
fixing multi-line viz issue of incorrectly formatted text for chart key when single line chart is used as a data
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before**
<img width="1017" alt="BeforeMultiLinVizFix" src="https://user-images.githubusercontent.com/20442310/106868364-4d1ef700-6694-11eb-8be1-f3d12764fadd.png">
**After**
<img width="1012" alt="AfterMultiLineVizFix" src="https://user-images.githubusercontent.com/20442310/106868365-4db78d80-6694-11eb-8abb-3b64a55764cc.png">
### TEST PLAN
Observe correctly formatted key for data in API call for both multi-line charts and single-line charts as a chart source
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #12935
- [x] Fixes #12935
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] 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.
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] rwspielman commented on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773549486
sorry - was making this into a single commit
----------------------------------------------------------------
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] [superset] codecov-io commented on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
# [Codecov](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=h1) Report
> Merging [#12937](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=desc) (65f3417) into [master](https://codecov.io/gh/apache/superset/commit/77093a874e8c4c81c6e3b8557bca545c3fc0f3a2?el=desc) (77093a8) will **decrease** coverage by `1.76%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12937 +/- ##
==========================================
- Coverage 68.98% 67.21% -1.77%
==========================================
Files 1025 489 -536
Lines 48767 28670 -20097
Branches 5241 0 -5241
==========================================
- Hits 33641 19271 -14370
+ Misses 14989 9399 -5590
+ Partials 137 0 -137
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.21% <0.00%> (-0.42%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.01% <0.00%> (-0.04%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-16.93%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.93%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.22% <0.00%> (-1.64%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `85.50% <0.00%> (-0.50%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.98% <0.00%> (-0.45%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.41% <0.00%> (-0.14%)` | :arrow_down: |
| ... and [536 more](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12937?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/superset/pull/12937?src=pr&el=footer). Last update [77093a8...65f3417](https://codecov.io/gh/apache/superset/pull/12937?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] [superset] codecov-io edited a comment on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
# [Codecov](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=h1) Report
> Merging [#12937](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=desc) (863536f) into [master](https://codecov.io/gh/apache/superset/commit/77093a874e8c4c81c6e3b8557bca545c3fc0f3a2?el=desc) (77093a8) will **decrease** coverage by `2.06%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12937 +/- ##
==========================================
- Coverage 68.98% 66.91% -2.07%
==========================================
Files 1025 489 -536
Lines 48767 28689 -20078
Branches 5241 0 -5241
==========================================
- Hits 33641 19198 -14443
+ Misses 14989 9491 -5498
+ Partials 137 0 -137
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.91% <0.00%> (-0.71%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.01% <0.00%> (-0.04%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.85% <0.00%> (-6.15%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-6.07%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [560 more](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12937?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/superset/pull/12937?src=pr&el=footer). Last update [77093a8...863536f](https://codecov.io/gh/apache/superset/pull/12937?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] [superset] codecov-io edited a comment on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
# [Codecov](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=h1) Report
> Merging [#12937](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=desc) (65f3417) into [master](https://codecov.io/gh/apache/superset/commit/77093a874e8c4c81c6e3b8557bca545c3fc0f3a2?el=desc) (77093a8) will **decrease** coverage by `8.05%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12937 +/- ##
==========================================
- Coverage 68.98% 60.92% -8.06%
==========================================
Files 1025 966 -59
Lines 48767 45888 -2879
Branches 5241 4444 -797
==========================================
- Hits 33641 27959 -5682
- Misses 14989 17929 +2940
+ Partials 137 0 -137
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.45% <ø> (+1.31%)` | :arrow_up: |
| javascript | `?` | |
| python | `67.21% <0.00%> (-0.42%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.01% <0.00%> (-0.04%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [405 more](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12937?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/superset/pull/12937?src=pr&el=footer). Last update [77093a8...65f3417](https://codecov.io/gh/apache/superset/pull/12937?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] [superset] codecov-io edited a comment on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
----------------------------------------------------------------
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] [superset] villebro merged pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #12937:
URL: https://github.com/apache/superset/pull/12937
----------------------------------------------------------------
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] [superset] rwspielman commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570413264
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as tuples or lists (looking at `viz_type="line"`) and defaults to an empty list if None on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain 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.
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] rwspielman commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570413264
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as `tuple` or `list` or `str` (looking at `viz_type="line"`) and defaults to an empty list if empty `list` or `tuple` on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain 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.
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] rwspielman commented on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773549486
sorry - was making this into a single commit
----------------------------------------------------------------
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] [superset] villebro closed pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
villebro closed pull request #12937:
URL: https://github.com/apache/superset/pull/12937
----------------------------------------------------------------
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] [superset] rwspielman commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570413264
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as tuples or lists (looking at `viz_type="line"`) and defaults to an empty list if None on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain this
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as `tuple` or `list` or `str` (looking at `viz_type="line"`) and defaults to an empty list if None on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain this
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as `tuple` or `list` or `str` (looking at `viz_type="line"`) and defaults to an empty list if empty `list` or `tuple` on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain this
##########
File path: superset/viz.py
##########
@@ -1456,7 +1456,7 @@ def get_data(self, df: pd.DataFrame) -> VizData:
data.append(
{
- "key": prefix + ", ".join(series["key"]),
+ "key": prefix + "".join(series["key"]),
Review comment:
Good idea, updated
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as `tuple` or `list` or `str` (looking at `viz_type="line"`) and defaults to an empty list if empty `list` or `tuple` or empty `str` on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain 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.
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-io commented on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
# [Codecov](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=h1) Report
> Merging [#12937](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=desc) (65f3417) into [master](https://codecov.io/gh/apache/superset/commit/77093a874e8c4c81c6e3b8557bca545c3fc0f3a2?el=desc) (77093a8) will **decrease** coverage by `1.76%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12937 +/- ##
==========================================
- Coverage 68.98% 67.21% -1.77%
==========================================
Files 1025 489 -536
Lines 48767 28670 -20097
Branches 5241 0 -5241
==========================================
- Hits 33641 19271 -14370
+ Misses 14989 9399 -5590
+ Partials 137 0 -137
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.21% <0.00%> (-0.42%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.01% <0.00%> (-0.04%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-16.93%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.93%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.22% <0.00%> (-1.64%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `85.50% <0.00%> (-0.50%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.98% <0.00%> (-0.45%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.41% <0.00%> (-0.14%)` | :arrow_down: |
| ... and [536 more](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12937?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/superset/pull/12937?src=pr&el=footer). Last update [77093a8...65f3417](https://codecov.io/gh/apache/superset/pull/12937?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] [superset] codecov-io edited a comment on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
# [Codecov](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=h1) Report
> Merging [#12937](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=desc) (65f3417) into [master](https://codecov.io/gh/apache/superset/commit/77093a874e8c4c81c6e3b8557bca545c3fc0f3a2?el=desc) (77093a8) will **decrease** coverage by `7.91%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12937 +/- ##
==========================================
- Coverage 68.98% 61.06% -7.92%
==========================================
Files 1025 966 -59
Lines 48767 45888 -2879
Branches 5241 4444 -797
==========================================
- Hits 33641 28023 -5618
- Misses 14989 17865 +2876
+ Partials 137 0 -137
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.83% <ø> (+1.68%)` | :arrow_up: |
| javascript | `?` | |
| python | `67.21% <0.00%> (-0.42%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.01% <0.00%> (-0.04%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [405 more](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12937?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/superset/pull/12937?src=pr&el=footer). Last update [77093a8...65f3417](https://codecov.io/gh/apache/superset/pull/12937?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] [superset] rwspielman commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570416001
##########
File path: superset/viz.py
##########
@@ -1456,7 +1456,7 @@ def get_data(self, df: pd.DataFrame) -> VizData:
data.append(
{
- "key": prefix + ", ".join(series["key"]),
+ "key": prefix + "".join(series["key"]),
Review comment:
Good idea, updated
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
# [Codecov](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=h1) Report
> Merging [#12937](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=desc) (65f3417) into [master](https://codecov.io/gh/apache/superset/commit/77093a874e8c4c81c6e3b8557bca545c3fc0f3a2?el=desc) (77093a8) will **decrease** coverage by `8.97%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12937 +/- ##
==========================================
- Coverage 68.98% 60.00% -8.98%
==========================================
Files 1025 966 -59
Lines 48767 45888 -2879
Branches 5241 4444 -797
==========================================
- Hits 33641 27535 -6106
- Misses 14989 18353 +3364
+ Partials 137 0 -137
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `47.99% <ø> (-1.15%)` | :arrow_down: |
| javascript | `?` | |
| python | `67.21% <0.00%> (-0.42%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.01% <0.00%> (-0.04%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...tend/src/dashboard/util/getDirectPathToTabIndex.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERpcmVjdFBhdGhUb1RhYkluZGV4Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...tend/src/visualizations/FilterBox/controlPanel.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL0ZpbHRlckJveC9jb250cm9sUGFuZWwuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rc/dashboard/util/getLayoutComponentFromChartId.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldExheW91dENvbXBvbmVudEZyb21DaGFydElkLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| ... and [423 more](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12937?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/superset/pull/12937?src=pr&el=footer). Last update [77093a8...65f3417](https://codecov.io/gh/apache/superset/pull/12937?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] [superset] rwspielman closed pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman closed pull request #12937:
URL: https://github.com/apache/superset/pull/12937
----------------------------------------------------------------
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] [superset] rwspielman closed pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman closed pull request #12937:
URL: https://github.com/apache/superset/pull/12937
----------------------------------------------------------------
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] [superset] villebro commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570063554
##########
File path: superset/viz.py
##########
@@ -1456,7 +1456,7 @@ def get_data(self, df: pd.DataFrame) -> VizData:
data.append(
{
- "key": prefix + ", ".join(series["key"]),
+ "key": prefix + "".join(series["key"]),
Review comment:
Good catch; I believe the key can be both `str` and `List[str]`. Due to `viz.py` not being actively maintained and being light on tests it might be a good idea to add some additional logic to guard against regressions. Perhaps we should do something like this (probably makes sense to break out `series["key"]` into a variable to avoid duplication):
```suggestion
"key": prefix + ", ".join(series["key"] if isinstance(series["key"], list) else [series["key"]]),
```
----------------------------------------------------------------
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] [superset] rwspielman commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570413264
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as `tuple` or `list` or `str` (looking at `viz_type="line"`) and defaults to an empty list if empty `list` or `tuple` or empty `str` on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain 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.
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-io edited a comment on pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12937:
URL: https://github.com/apache/superset/pull/12937#issuecomment-773489163
# [Codecov](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=h1) Report
> Merging [#12937](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=desc) (863536f) into [master](https://codecov.io/gh/apache/superset/commit/77093a874e8c4c81c6e3b8557bca545c3fc0f3a2?el=desc) (77093a8) will **increase** coverage by `0.16%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12937 +/- ##
==========================================
+ Coverage 68.98% 69.14% +0.16%
==========================================
Files 1025 494 -531
Lines 48767 32606 -16161
Branches 5241 0 -5241
==========================================
- Hits 33641 22547 -11094
+ Misses 14989 10059 -4930
+ Partials 137 0 -137
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `69.14% <0.00%> (+1.52%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12937?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `65.12% <0.00%> (+6.07%)` | :arrow_up: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `94.73% <0.00%> (-5.27%)` | :arrow_down: |
| ... and [589 more](https://codecov.io/gh/apache/superset/pull/12937/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12937?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/superset/pull/12937?src=pr&el=footer). Last update [77093a8...863536f](https://codecov.io/gh/apache/superset/pull/12937?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] [superset] villebro commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570063554
##########
File path: superset/viz.py
##########
@@ -1456,7 +1456,7 @@ def get_data(self, df: pd.DataFrame) -> VizData:
data.append(
{
- "key": prefix + ", ".join(series["key"]),
+ "key": prefix + "".join(series["key"]),
Review comment:
Good catch; I believe the key can be both `str` and `List[str]`. Due to `viz.py` not being actively maintained and being light on tests it might be a good idea to add some additional logic to guard against regressions. Perhaps we should do something like this (probably makes sense to break out `series["key"]` into a variable to avoid duplication):
```suggestion
"key": prefix + ", ".join(series["key"] if isinstance(series["key"], list) else [series["key"]]),
```
----------------------------------------------------------------
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] [superset] rwspielman commented on a change in pull request #12937: fix: chart keys in MultiLineViz
Posted by GitBox <gi...@apache.org>.
rwspielman commented on a change in pull request #12937:
URL: https://github.com/apache/superset/pull/12937#discussion_r570413264
##########
File path: superset/viz.py
##########
@@ -1453,10 +1453,10 @@ def get_data(self, df: pd.DataFrame) -> VizData:
x_values = [value["x"] for value in series["values"]]
min_x = min(x_values + ([min_x] if min_x is not None else []))
max_x = max(x_values + ([max_x] if max_x is not None else []))
-
+ series_keys = series["key"] if isinstance(series["key"], (list, tuple)) else [series["key"]]
Review comment:
_looks_ like they come in as `tuple` or `list` or `str` (looking at `viz_type="line"`) and defaults to an empty list if None on line 1451.
I don't want to change much as I'm not very familiar with the codebase yet so sorry this is minimal effort. Hopefully, at some point, I can come back and help maintain 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.
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