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/09/29 09:07:47 UTC

[GitHub] [incubator-superset] villebro opened a new pull request #11103: fix: echarts timeseries groupby

villebro opened a new pull request #11103:
URL: https://github.com/apache/incubator-superset/pull/11103


   ### SUMMARY
   Bump ECharts viz plugin to `0.13.4` to fix a bug affecting the groupby control (https://github.com/apache-superset/superset-ui/pull/800). Also add support for pivoting non-string column values.
   
   ### TEST PLAN
   Local testing + CI + new tests
   
   ### 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
   


----------------------------------------------------------------
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 merged pull request #11103: fix: echarts timeseries groupby

Posted by GitBox <gi...@apache.org>.
villebro merged pull request #11103:
URL: https://github.com/apache/incubator-superset/pull/11103


   


----------------------------------------------------------------
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-commenter commented on pull request #11103: fix: echarts timeseries groupby

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #11103:
URL: https://github.com/apache/incubator-superset/pull/11103#issuecomment-700583079


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11103?src=pr&el=h1) Report
   > Merging [#11103](https://codecov.io/gh/apache/incubator-superset/pull/11103?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/8b458ac17297fc2af2f4bba703b097425f4e7414?el=desc) will **decrease** coverage by `0.83%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11103/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11103?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11103      +/-   ##
   ==========================================
   - Coverage   61.36%   60.52%   -0.84%     
   ==========================================
     Files         383      383              
     Lines       24188    24186       -2     
   ==========================================
   - Hits        14842    14638     -204     
   - Misses       9346     9548     +202     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #python | `60.52% <100.00%> (-0.84%)` | :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/incubator-superset/pull/11103?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/charts/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY2hhcnRzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `77.22% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/views/database/views.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.30% <0.00%> (-25.14%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.27% <0.00%> (-8.69%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `82.97% <0.00%> (-8.52%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `80.05% <0.00%> (-7.28%)` | :arrow_down: |
   | [superset/sql\_validators/base.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
   | [superset/views/database/forms.py](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
   | ... and [12 more](https://codecov.io/gh/apache/incubator-superset/pull/11103/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11103?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/11103?src=pr&el=footer). Last update [8b458ac...c059d1f](https://codecov.io/gh/apache/incubator-superset/pull/11103?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] villebro commented on a change in pull request #11103: fix: echarts timeseries groupby

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11103:
URL: https://github.com/apache/incubator-superset/pull/11103#discussion_r496561535



##########
File path: superset/utils/pandas_postprocessing.py
##########
@@ -106,15 +107,13 @@ def _flatten_column_after_pivot(
     :param aggregates: aggregates
     :return:
     """
-    if isinstance(column, str):
-        return column
-    if len(column) == 1:
-        return column[0]
+    if not isinstance(column, tuple):
+        column = (column,)
     if len(aggregates) == 1 and len(column) > 1:
         # drop aggregate for single aggregate pivots with multiple groupings
         # from column name (aggregates always come first in column name)
         column = column[1:]
-    return ", ".join(column)
+    return ", ".join([str(col) for col in column])

Review comment:
       It's a shame we have to format everything as strings for now, but hoping to remedy this once we move on to some other serialization format than 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 #11103: fix: echarts timeseries groupby

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11103:
URL: https://github.com/apache/incubator-superset/pull/11103#discussion_r496560851



##########
File path: superset/charts/schemas.py
##########
@@ -718,6 +718,7 @@ class ChartDataQueryObjectSchema(Schema):
     )
     groupby = fields.List(
         fields.String(description="Columns by which to group the query.",),
+        allow_none=True,

Review comment:
       `QueryObject` automatically does `self.groupby = groupby or []` in the constructor, so no need to require a `fields.List`.




----------------------------------------------------------------
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 #11103: fix: echarts timeseries groupby

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11103:
URL: https://github.com/apache/incubator-superset/pull/11103#discussion_r496561021



##########
File path: superset/charts/schemas.py
##########
@@ -782,7 +783,7 @@ class ChartDataQueryObjectSchema(Schema):
         description="Reverse order. Default: `false`", required=False
     )
     extras = fields.Nested(ChartDataExtrasSchema, required=False)
-    columns = fields.List(fields.String(), description="",)
+    columns = fields.List(fields.String(), description="", allow_none=True)

Review comment:
       Same for `columns` as `groupby`.




----------------------------------------------------------------
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] dpgaspar commented on a change in pull request #11103: fix: echarts timeseries groupby

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #11103:
URL: https://github.com/apache/incubator-superset/pull/11103#discussion_r496585581



##########
File path: superset/charts/schemas.py
##########
@@ -782,7 +783,7 @@ class ChartDataQueryObjectSchema(Schema):
         description="Reverse order. Default: `false`", required=False
     )
     extras = fields.Nested(ChartDataExtrasSchema, required=False)
-    columns = fields.List(fields.String(), description="",)
+    columns = fields.List(fields.String(), description="", allow_none=True)

Review comment:
       nit could be nice to add a description to `columns` and `extras`




----------------------------------------------------------------
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