You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/01/14 11:55:37 UTC

[GitHub] [superset] zhaoyongjie opened a new pull request #18045: feat(advanced analytics): support groupby in resample

zhaoyongjie opened a new pull request #18045:
URL: https://github.com/apache/superset/pull/18045


   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   TBD
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18045:
URL: https://github.com/apache/superset/pull/18045#discussion_r785793505



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       I have created a Shotcut ticket to record this.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18045:
URL: https://github.com/apache/superset/pull/18045#discussion_r784803988



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       I think the `sqlExpression` can't fit in `pandas` column, so I deliberately let it produce `undefine`.
   
   ```
   ....
   export default function getColumnLabel(column: QueryFormColumn): string {
     if (isPhysicalColumn(column)) {
       return column;
     }
     if (column.label) {
       return column.label;
     }
     return column.sqlExpression;
   }
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8f7c6) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   - Coverage   66.34%   66.34%   -0.01%     
   ==========================================
     Files        1569     1569              
     Lines       61685    61691       +6     
     Branches     6240     6240              
   ==========================================
   + Hits        40927    40929       +2     
   - Misses      19161    19165       +4     
     Partials     1597     1597              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `82.09% <100.00%> (+<0.01%)` | :arrow_up: |
   | postgres | `82.13% <100.00%> (-0.01%)` | :arrow_down: |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.58% <100.00%> (-0.01%)` | :arrow_down: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
   | [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
   | [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.08% <0.00%> (-0.55%)` | :arrow_down: |
   | [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `91.14% <0.00%> (-0.37%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `97.59% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.12% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.27% <0.00%> (ø)` | |
   | ... and [3 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...1f8f7c6](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c25cd2) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   - Coverage   66.34%   66.33%   -0.02%     
   ==========================================
     Files        1569     1569              
     Lines       61685    61691       +6     
     Branches     6240     6240              
   ==========================================
   - Hits        40927    40921       -6     
   - Misses      19161    19173      +12     
     Partials     1597     1597              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `?` | |
   | postgres | `82.15% <100.00%> (+<0.01%)` | :arrow_up: |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.55% <100.00%> (-0.04%)` | :arrow_down: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `93.97% <0.00%> (-3.65%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.75% <0.00%> (-0.74%)` | :arrow_down: |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `77.29% <0.00%> (-0.45%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.12% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.27% <0.00%> (ø)` | |
   | ... and [1 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...7c25cd2](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie merged pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie merged pull request #18045:
URL: https://github.com/apache/superset/pull/18045


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8f7c6) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 1f8f7c6 differs from pull request most recent head 67554ca. Consider uploading reports for the commit 67554ca to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #18045   +/-   ##
   =======================================
     Coverage   66.34%   66.35%           
   =======================================
     Files        1569     1569           
     Lines       61685    61697   +12     
     Branches     6240     6242    +2     
   =======================================
   + Hits        40927    40937   +10     
   - Misses      19161    19162    +1     
   - Partials     1597     1598    +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | javascript | `50.92% <100.00%> (+<0.01%)` | :arrow_up: |
   | mysql | `82.09% <100.00%> (+<0.01%)` | :arrow_up: |
   | postgres | `82.15% <100.00%> (+<0.01%)` | :arrow_up: |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.59% <100.00%> (+<0.01%)` | :arrow_up: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `50.73% <0.00%> (-0.26%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `97.59% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.12% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.27% <0.00%> (ø)` | |
   | [...rontend/src/visualizations/TimeTable/TimeTable.jsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL1RpbWVUYWJsZS9UaW1lVGFibGUuanN4) | `0.00% <0.00%> (ø)` | |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.78% <0.00%> (+0.04%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...67554ca](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18045:
URL: https://github.com/apache/superset/pull/18045#discussion_r785256468



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       > @zhaoyongjie maybe we should change the label to an MD5 or some other hash if `column.label` is falsy? I think for the most part we don't need the label to be equal to `sqlExpression`, but rather to just make sure it's deterministic.
   
   IMO, we have to give every adhoc column a explicit and unique name. This name not only used in `alias` in the sql, but also in pandas column.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a738225) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **decrease** coverage by `0.00%`.
   > The diff coverage is `80.89%`.
   
   > :exclamation: Current head a738225 differs from pull request most recent head 67554ca. Consider uploading reports for the commit 67554ca to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   - Coverage   66.34%   66.34%   -0.01%     
   ==========================================
     Files        1569     1570       +1     
     Lines       61685    61746      +61     
     Branches     6240     6241       +1     
   ==========================================
   + Hits        40927    40966      +39     
   - Misses      19161    19182      +21     
   - Partials     1597     1598       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.21% <58.64%> (-0.03%)` | :arrow_down: |
   | mysql | `?` | |
   | postgres | `82.15% <84.21%> (+<0.01%)` | :arrow_up: |
   | presto | `53.05% <59.39%> (-0.03%)` | :arrow_down: |
   | python | `82.55% <84.21%> (-0.04%)` | :arrow_down: |
   | sqlite | `81.84% <84.21%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `50.73% <0.00%> (-0.26%)` | :arrow_down: |
   | [...rontend/src/visualizations/TimeTable/TimeTable.jsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL1RpbWVUYWJsZS9UaW1lVGFibGUuanN4) | `0.00% <0.00%> (ø)` | |
   | [superset/utils/mock\_data.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvbW9ja19kYXRhLnB5) | `25.18% <0.00%> (ø)` | |
   | [superset/config.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.71% <25.00%> (+0.02%)` | :arrow_up: |
   | [superset/cli.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2xpLnB5) | `54.19% <50.00%> (ø)` | |
   | [superset/examples/country\_map.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvY291bnRyeV9tYXAucHk=) | `23.80% <50.00%> (ø)` | |
   | [superset/examples/energy.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvZW5lcmd5LnB5) | `25.00% <50.00%> (ø)` | |
   | [superset/examples/flights.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvZmxpZ2h0cy5weQ==) | `17.64% <50.00%> (ø)` | |
   | [superset/examples/long\_lat.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvbG9uZ19sYXQucHk=) | `22.72% <50.00%> (ø)` | |
   | ... and [24 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...67554ca](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a738225) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **increase** coverage by `0.01%`.
   > The diff coverage is `80.89%`.
   
   > :exclamation: Current head a738225 differs from pull request most recent head 67554ca. Consider uploading reports for the commit 67554ca to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   + Coverage   66.34%   66.36%   +0.01%     
   ==========================================
     Files        1569     1570       +1     
     Lines       61685    61746      +61     
     Branches     6240     6241       +1     
   ==========================================
   + Hits        40927    40978      +51     
   - Misses      19161    19170       +9     
   - Partials     1597     1598       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.21% <58.64%> (-0.03%)` | :arrow_down: |
   | mysql | `82.10% <84.21%> (+<0.01%)` | :arrow_up: |
   | postgres | `82.15% <84.21%> (+<0.01%)` | :arrow_up: |
   | presto | `53.05% <59.39%> (-0.03%)` | :arrow_down: |
   | python | `82.59% <84.21%> (+<0.01%)` | :arrow_up: |
   | sqlite | `81.84% <84.21%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `50.73% <0.00%> (-0.26%)` | :arrow_down: |
   | [...rontend/src/visualizations/TimeTable/TimeTable.jsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL1RpbWVUYWJsZS9UaW1lVGFibGUuanN4) | `0.00% <0.00%> (ø)` | |
   | [superset/utils/mock\_data.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvbW9ja19kYXRhLnB5) | `25.18% <0.00%> (ø)` | |
   | [superset/config.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.71% <25.00%> (+0.02%)` | :arrow_up: |
   | [superset/cli.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2xpLnB5) | `54.19% <50.00%> (ø)` | |
   | [superset/examples/country\_map.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvY291bnRyeV9tYXAucHk=) | `23.80% <50.00%> (ø)` | |
   | [superset/examples/energy.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvZW5lcmd5LnB5) | `25.00% <50.00%> (ø)` | |
   | [superset/examples/flights.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvZmxpZ2h0cy5weQ==) | `17.64% <50.00%> (ø)` | |
   | [superset/examples/long\_lat.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvbG9uZ19sYXQucHk=) | `22.72% <50.00%> (ø)` | |
   | ... and [22 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...67554ca](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18045:
URL: https://github.com/apache/superset/pull/18045#discussion_r785278614



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       @villebro @kgabryje 
   Could we migrate this PR first? After improve the ad-hoc column logic, I will remove the redundant cods.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18045:
URL: https://github.com/apache/superset/pull/18045#discussion_r785278614



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       @villebro @kgabryje 
   Could we migrate this PR first? after improve the ad-hoc column logic, I will remove the redundant cods.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8f7c6) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #18045   +/-   ##
   =======================================
     Coverage   66.34%   66.35%           
   =======================================
     Files        1569     1569           
     Lines       61685    61691    +6     
     Branches     6240     6240           
   =======================================
   + Hits        40927    40933    +6     
     Misses      19161    19161           
     Partials     1597     1597           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `82.09% <100.00%> (+<0.01%)` | :arrow_up: |
   | postgres | `82.15% <100.00%> (+<0.01%)` | :arrow_up: |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.59% <100.00%> (+<0.01%)` | :arrow_up: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `97.59% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.12% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.27% <0.00%> (ø)` | |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.78% <0.00%> (+0.04%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...1f8f7c6](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie removed a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie removed a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013095193


   IMO, we have to give every adhoc column a explicit and unique name. This
   name not only used in ‘alias’ in the sql, but also in pandas column.
   
   On Fri, Jan 14, 2022 at 20:43 Ville Brofeldt ***@***.***>
   wrote:
   
   > ***@***.**** commented on this pull request.
   > ------------------------------
   >
   > In
   > superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
   > <https://github.com/apache/superset/pull/18045#discussion_r784813929>:
   >
   > > +    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
   > +      if (isPhysicalColumn(column)) {
   > +        return column;
   > +      }
   > +      return column.label;
   > +    });
   >
   > @zhaoyongjie <https://github.com/zhaoyongjie> btw I just noticed that
   > when creating two adhoc columns with *different* expressions where the
   > label is *unchanged*, the query only includes the first one:
   > [image: image]
   > <https://user-images.githubusercontent.com/33317356/149516917-31ecd51b-ec44-4d1f-9291-7114fe88ec86.png>
   > So we should perhaps assign the label in the popover, even just giving
   > them a unique serial id, like "My Column", "My Column (1)", ..., "My Column
   > (n)".
   >
   > Ping @kgabryje <https://github.com/kgabryje>
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/superset/pull/18045#discussion_r784813929>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AAPMKUVPIAHNCREU4R5G7EDUWAK7BANCNFSM5L6TO4FQ>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   -- 
   
   Best regards,
   
   Yongjie
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

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



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       @zhaoyongjie maybe we should change the label to an MD5 or some other hash if `column.label` is falsy? I think for the most part we don't need the label to be equal to `sqlExpression`, but rather to just make sure it's deterministic.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

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



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       @zhaoyongjie I'm ok with this, but I'd recommend following up on this quickly, as we're now in a kind-of bad place with stacked tech debt due to this:
   - possibility of duplicate column labels
   - needed workaround due to `getColumnLabel` not being universally applicable due to the label property being optional
   - the type `groupby_columns?: Array<string | undefined>;` which IMO should just be `groupby_columns?: string[]`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

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



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       @zhaoyongjie I'm ok with this, but I'd recommend following up on this quickly, as we're now in a kind-of bad place with stacked tech debt due to this:
   - possibility of duplicate column labels
   - needed workaround due to `getColumnLabel` not being universally applicable due to the label property being optional
   - the type `groupby_columns?: Array<string | undefined>` which IMO should just be `groupby_columns?: string[]`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8f7c6) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **decrease** coverage by `0.02%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   - Coverage   66.34%   66.32%   -0.03%     
   ==========================================
     Files        1569     1569              
     Lines       61685    61691       +6     
     Branches     6240     6240              
   ==========================================
   - Hits        40927    40917      -10     
   - Misses      19161    19177      +16     
     Partials     1597     1597              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `82.09% <100.00%> (+<0.01%)` | :arrow_up: |
   | postgres | `?` | |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.54% <100.00%> (-0.05%)` | :arrow_down: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
   | [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `95.45% <0.00%> (-1.82%)` | :arrow_down: |
   | [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.08% <0.00%> (-0.55%)` | :arrow_down: |
   | [superset/views/base\_api.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.77% <0.00%> (-0.45%)` | :arrow_down: |
   | [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `91.14% <0.00%> (-0.37%)` | :arrow_down: |
   | ... and [5 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...1f8f7c6](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c25cd2) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 7c25cd2 differs from pull request most recent head bdd6fb9. Consider uploading reports for the commit bdd6fb9 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #18045   +/-   ##
   =======================================
     Coverage   66.34%   66.35%           
   =======================================
     Files        1569     1569           
     Lines       61685    61691    +6     
     Branches     6240     6240           
   =======================================
   + Hits        40927    40933    +6     
     Misses      19161    19161           
     Partials     1597     1597           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `82.09% <100.00%> (+<0.01%)` | :arrow_up: |
   | postgres | `82.15% <100.00%> (+<0.01%)` | :arrow_up: |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.59% <100.00%> (+<0.01%)` | :arrow_up: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `97.59% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.12% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.27% <0.00%> (ø)` | |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.78% <0.00%> (+0.04%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...bdd6fb9](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8f7c6) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #18045   +/-   ##
   =======================================
     Coverage   66.34%   66.35%           
   =======================================
     Files        1569     1569           
     Lines       61685    61697   +12     
     Branches     6240     6242    +2     
   =======================================
   + Hits        40927    40937   +10     
   - Misses      19161    19162    +1     
   - Partials     1597     1598    +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | javascript | `50.92% <100.00%> (+<0.01%)` | :arrow_up: |
   | mysql | `82.09% <100.00%> (+<0.01%)` | :arrow_up: |
   | postgres | `82.15% <100.00%> (+<0.01%)` | :arrow_up: |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.59% <100.00%> (+<0.01%)` | :arrow_up: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `50.73% <0.00%> (-0.26%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `97.59% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.12% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.27% <0.00%> (ø)` | |
   | [...rontend/src/visualizations/TimeTable/TimeTable.jsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL1RpbWVUYWJsZS9UaW1lVGFibGUuanN4) | `0.00% <0.00%> (ø)` | |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.78% <0.00%> (+0.04%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...1f8f7c6](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

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



##########
File path: tests/integration_tests/pandas_postprocessing_tests.py
##########
@@ -1029,3 +1029,70 @@ def test_resample(self):
         )
         self.assertListEqual(post_df["label"].tolist(), ["x", "y", 0, 0, "z", 0, "q"])
         self.assertListEqual(post_df["y"].tolist(), [1.0, 2.0, 0, 0, 3.0, 0, 4.0])
+
+    def test_resample_with_groupby(self):
+        """
+The Dataframe contains a timestamp column, a string column and a numeric column.
+  __timestamp     city  val
+0  2022-01-13  Chicago  6.0
+1  2022-01-13       LA  5.0
+2  2022-01-13       NY  4.0
+3  2022-01-11  Chicago  3.0
+4  2022-01-11       LA  2.0
+5  2022-01-11       NY  1.0
+        """
+        df = DataFrame(
+            {
+                "__timestamp": to_datetime(
+                    [
+                        "2022-01-13",
+                        "2022-01-13",
+                        "2022-01-13",
+                        "2022-01-11",
+                        "2022-01-11",
+                        "2022-01-11",
+                    ]
+                ),
+                "city": ["Chicago", "LA", "NY", "Chicago", "LA", "NY"],
+                "val": [6.0, 5.0, 4.0, 3.0, 2.0, 1.0],
+            }
+        )
+        post_df = proc.resample(
+            df=df,
+            rule="1D",
+            method="asfreq",
+            fill_value=0,
+            time_column="__timestamp",
+            groupby_columns=tuple(["city"]),

Review comment:
       nit:
   ```suggestion
               groupby_columns=("city",),
   ```

##########
File path: tests/integration_tests/pandas_postprocessing_tests.py
##########
@@ -1029,3 +1029,70 @@ def test_resample(self):
         )
         self.assertListEqual(post_df["label"].tolist(), ["x", "y", 0, 0, "z", 0, "q"])
         self.assertListEqual(post_df["y"].tolist(), [1.0, 2.0, 0, 0, 3.0, 0, 4.0])
+
+    def test_resample_with_groupby(self):
+        """
+The Dataframe contains a timestamp column, a string column and a numeric column.
+  __timestamp     city  val
+0  2022-01-13  Chicago  6.0
+1  2022-01-13       LA  5.0
+2  2022-01-13       NY  4.0
+3  2022-01-11  Chicago  3.0
+4  2022-01-11       LA  2.0
+5  2022-01-11       NY  1.0
+        """
+        df = DataFrame(
+            {
+                "__timestamp": to_datetime(
+                    [
+                        "2022-01-13",
+                        "2022-01-13",
+                        "2022-01-13",
+                        "2022-01-11",
+                        "2022-01-11",
+                        "2022-01-11",
+                    ]
+                ),
+                "city": ["Chicago", "LA", "NY", "Chicago", "LA", "NY"],
+                "val": [6.0, 5.0, 4.0, 3.0, 2.0, 1.0],
+            }
+        )
+        post_df = proc.resample(
+            df=df,
+            rule="1D",
+            method="asfreq",
+            fill_value=0,
+            time_column="__timestamp",
+            groupby_columns=tuple(["city"]),
+        )
+        assert list(post_df.columns) == [
+            "__timestamp",
+            "city",
+            "val",
+        ]
+        assert [str(dt.date()) for dt in post_df["__timestamp"]] == (
+            ["2022-01-11"] * 3 + ["2022-01-12"] * 3 + ["2022-01-13"] * 3
+        )
+        assert list(post_df["val"]) == [3.0, 2.0, 1.0, 0, 0, 0, 6.0, 5.0, 4.0]
+
+        # should raise error when get a non-existent column
+        with pytest.raises(QueryObjectValidationError):
+            proc.resample(
+                df=df,
+                rule="1D",
+                method="asfreq",
+                fill_value=0,
+                time_column="__timestamp",
+                groupby_columns=tuple(["city", "unkonw_column"]),

Review comment:
       nit:
   ```suggestion
                   groupby_columns=("city", "unkonw_column"),
   ```

##########
File path: tests/integration_tests/pandas_postprocessing_tests.py
##########
@@ -1029,3 +1029,70 @@ def test_resample(self):
         )
         self.assertListEqual(post_df["label"].tolist(), ["x", "y", 0, 0, "z", 0, "q"])
         self.assertListEqual(post_df["y"].tolist(), [1.0, 2.0, 0, 0, 3.0, 0, 4.0])
+
+    def test_resample_with_groupby(self):
+        """
+The Dataframe contains a timestamp column, a string column and a numeric column.
+  __timestamp     city  val
+0  2022-01-13  Chicago  6.0
+1  2022-01-13       LA  5.0
+2  2022-01-13       NY  4.0
+3  2022-01-11  Chicago  3.0
+4  2022-01-11       LA  2.0
+5  2022-01-11       NY  1.0
+        """
+        df = DataFrame(
+            {
+                "__timestamp": to_datetime(
+                    [
+                        "2022-01-13",
+                        "2022-01-13",
+                        "2022-01-13",
+                        "2022-01-11",
+                        "2022-01-11",
+                        "2022-01-11",
+                    ]
+                ),
+                "city": ["Chicago", "LA", "NY", "Chicago", "LA", "NY"],
+                "val": [6.0, 5.0, 4.0, 3.0, 2.0, 1.0],
+            }
+        )
+        post_df = proc.resample(
+            df=df,
+            rule="1D",
+            method="asfreq",
+            fill_value=0,
+            time_column="__timestamp",
+            groupby_columns=tuple(["city"]),
+        )
+        assert list(post_df.columns) == [
+            "__timestamp",
+            "city",
+            "val",
+        ]
+        assert [str(dt.date()) for dt in post_df["__timestamp"]] == (
+            ["2022-01-11"] * 3 + ["2022-01-12"] * 3 + ["2022-01-13"] * 3
+        )
+        assert list(post_df["val"]) == [3.0, 2.0, 1.0, 0, 0, 0, 6.0, 5.0, 4.0]
+
+        # should raise error when get a non-existent column
+        with pytest.raises(QueryObjectValidationError):
+            proc.resample(
+                df=df,
+                rule="1D",
+                method="asfreq",
+                fill_value=0,
+                time_column="__timestamp",
+                groupby_columns=tuple(["city", "unkonw_column"]),
+            )
+
+        # should raise error when get a None value in groupby list
+        with pytest.raises(QueryObjectValidationError):
+            proc.resample(
+                df=df,
+                rule="1D",
+                method="asfreq",
+                fill_value=0,
+                time_column="__timestamp",
+                groupby_columns=tuple(["city", None]),

Review comment:
       nit:
   ```suggestion
                   groupby_columns=("city", None),
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a738225) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **decrease** coverage by `0.02%`.
   > The diff coverage is `80.89%`.
   
   > :exclamation: Current head a738225 differs from pull request most recent head 67554ca. Consider uploading reports for the commit 67554ca to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   - Coverage   66.34%   66.32%   -0.03%     
   ==========================================
     Files        1569     1570       +1     
     Lines       61685    61746      +61     
     Branches     6240     6241       +1     
   ==========================================
   + Hits        40927    40953      +26     
   - Misses      19161    19195      +34     
   - Partials     1597     1598       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.21% <58.64%> (-0.03%)` | :arrow_down: |
   | mysql | `?` | |
   | postgres | `82.15% <84.21%> (+<0.01%)` | :arrow_up: |
   | presto | `53.05% <59.39%> (-0.03%)` | :arrow_down: |
   | python | `82.51% <84.21%> (-0.08%)` | :arrow_down: |
   | sqlite | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `50.73% <0.00%> (-0.26%)` | :arrow_down: |
   | [...rontend/src/visualizations/TimeTable/TimeTable.jsx](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL1RpbWVUYWJsZS9UaW1lVGFibGUuanN4) | `0.00% <0.00%> (ø)` | |
   | [superset/utils/mock\_data.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvbW9ja19kYXRhLnB5) | `25.18% <0.00%> (ø)` | |
   | [superset/config.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.71% <25.00%> (+0.02%)` | :arrow_up: |
   | [superset/cli.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2xpLnB5) | `54.19% <50.00%> (ø)` | |
   | [superset/examples/country\_map.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvY291bnRyeV9tYXAucHk=) | `23.80% <50.00%> (ø)` | |
   | [superset/examples/energy.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvZW5lcmd5LnB5) | `25.00% <50.00%> (ø)` | |
   | [superset/examples/flights.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvZmxpZ2h0cy5weQ==) | `17.64% <50.00%> (ø)` | |
   | [superset/examples/long\_lat.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhhbXBsZXMvbG9uZ19sYXQucHk=) | `22.72% <50.00%> (ø)` | |
   | ... and [32 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...67554ca](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #18045:
URL: https://github.com/apache/superset/pull/18045#discussion_r784804335



##########
File path: superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts
##########
@@ -167,6 +167,9 @@ export interface PostProcessingResample {
     rule: string;
     fill_value?: number | null;
     time_column: string;
+    // If AdhocColumn doesn't have a label, it will be undefined.
+    // todo: we have to give an explicit label for AdhocColumn.
+    groupby_columns?: Array<string | undefined>;

Review comment:
       same above




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c25cd2) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **decrease** coverage by `0.18%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   - Coverage   66.34%   66.16%   -0.19%     
   ==========================================
     Files        1569     1569              
     Lines       61685    61691       +6     
     Branches     6240     6240              
   ==========================================
   - Hits        40927    40818     -109     
   - Misses      19161    19276     +115     
     Partials     1597     1597              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.21% <100.00%> (-0.38%)` | :arrow_down: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.37% <0.00%> (-56.87%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `60.34% <0.00%> (-20.69%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.10% <0.00%> (-5.93%)` | :arrow_down: |
   | [superset/databases/dao.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
   | ... and [18 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...7c25cd2](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

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



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       @zhaoyongjie btw I just noticed that when creating two adhoc columns with _different_ expressions where the label is _unchanged_, the query only includes the first one:
   ![image](https://user-images.githubusercontent.com/33317356/149516917-31ecd51b-ec44-4d1f-9291-7114fe88ec86.png)
   So we should perhaps assign the label in the popover, even just giving them a unique serial id, like "My Column", "My Column (1)", ..., "My Column (n)".
   
   Ping @kgabryje 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

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



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       > I like the idea, it shouldn't be difficult to implement. Also, what do you think about creating a validator that returns an error when 2 or more columns or metrics have the same label?
   
   Yeah! That would make it bullet proof 👍




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #18045:
URL: https://github.com/apache/superset/pull/18045#discussion_r784826798



##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       I like the idea, it shouldn't be difficult to implement. Also, what do you think about creating a validator that returns an error when 2 or more columns or metrics have the same label?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7c25cd2) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #18045   +/-   ##
   =======================================
     Coverage   66.34%   66.35%           
   =======================================
     Files        1569     1569           
     Lines       61685    61691    +6     
     Branches     6240     6240           
   =======================================
   + Hits        40927    40933    +6     
     Misses      19161    19161           
     Partials     1597     1597           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `82.09% <100.00%> (+<0.01%)` | :arrow_up: |
   | postgres | `82.15% <100.00%> (+<0.01%)` | :arrow_up: |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.59% <100.00%> (+<0.01%)` | :arrow_up: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `97.59% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `89.12% <0.00%> (-0.03%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.27% <0.00%> (ø)` | |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.78% <0.00%> (+0.04%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...7c25cd2](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] zhaoyongjie commented on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013095193


   IMO, we have to give every adhoc column a explicit and unique name. This
   name not only used in ‘alias’ in the sql, but also in pandas column.
   
   On Fri, Jan 14, 2022 at 20:43 Ville Brofeldt ***@***.***>
   wrote:
   
   > ***@***.**** commented on this pull request.
   > ------------------------------
   >
   > In
   > superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
   > <https://github.com/apache/superset/pull/18045#discussion_r784813929>:
   >
   > > +    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
   > +      if (isPhysicalColumn(column)) {
   > +        return column;
   > +      }
   > +      return column.label;
   > +    });
   >
   > @zhaoyongjie <https://github.com/zhaoyongjie> btw I just noticed that
   > when creating two adhoc columns with *different* expressions where the
   > label is *unchanged*, the query only includes the first one:
   > [image: image]
   > <https://user-images.githubusercontent.com/33317356/149516917-31ecd51b-ec44-4d1f-9291-7114fe88ec86.png>
   > So we should perhaps assign the label in the popover, even just giving
   > them a unique serial id, like "My Column", "My Column (1)", ..., "My Column
   > (n)".
   >
   > Ping @kgabryje <https://github.com/kgabryje>
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/superset/pull/18045#discussion_r784813929>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AAPMKUVPIAHNCREU4R5G7EDUWAK7BANCNFSM5L6TO4FQ>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   -- 
   
   Best regards,
   
   Yongjie
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #18045: feat(advanced analytics): support groupby in resample

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18045:
URL: https://github.com/apache/superset/pull/18045#issuecomment-1013082129


   # [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18045](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1f8f7c6) into [master](https://codecov.io/gh/apache/superset/commit/14b9298ef72e73372c2d3f3b1f9f5a1cfb064e1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14b9298) will **decrease** coverage by `0.18%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/18045/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #18045      +/-   ##
   ==========================================
   - Coverage   66.34%   66.16%   -0.19%     
   ==========================================
     Files        1569     1569              
     Lines       61685    61691       +6     
     Branches     6240     6240              
   ==========================================
   - Hits        40927    40818     -109     
   - Misses      19161    19276     +115     
     Partials     1597     1597              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `53.23% <18.18%> (-0.01%)` | :arrow_down: |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `53.07% <18.18%> (-0.01%)` | :arrow_down: |
   | python | `82.21% <100.00%> (-0.38%)` | :arrow_down: |
   | sqlite | `81.83% <100.00%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...superset-ui-core/src/query/types/PostProcessing.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvcXVlcnkvdHlwZXMvUG9zdFByb2Nlc3NpbmcudHM=) | `100.00% <ø> (ø)` | |
   | [...i-chart-controls/src/operators/resampleOperator.ts](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL29wZXJhdG9ycy9yZXNhbXBsZU9wZXJhdG9yLnRz) | `100.00% <100.00%> (ø)` | |
   | [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `86.07% <100.00%> (+0.26%)` | :arrow_up: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.37% <0.00%> (-56.87%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `60.34% <0.00%> (-20.69%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.10% <0.00%> (-5.93%)` | :arrow_down: |
   | [superset/databases/dao.py](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
   | ... and [18 more](https://codecov.io/gh/apache/superset/pull/18045/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [14b9298...1f8f7c6](https://codecov.io/gh/apache/superset/pull/18045?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a change in pull request #18045: feat(advanced analytics): support groupby in resample

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



##########
File path: superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts
##########
@@ -167,6 +167,9 @@ export interface PostProcessingResample {
     rule: string;
     fill_value?: number | null;
     time_column: string;
+    // If AdhocColumn doesn't have a label, it will be undefined.
+    // todo: we have to give an explicit label for AdhocColumn.
+    groupby_columns?: Array<string | undefined>;

Review comment:
       Can't this type just be `groupby_columns?: string[];`?

##########
File path: superset-frontend/packages/superset-ui-chart-controls/src/operators/resampleOperator.ts
##########
@@ -28,13 +32,21 @@ export const resampleOperator: PostProcessingFactory<
   const resampleMethod = resampleZeroFill ? 'asfreq' : formData.resample_method;
   const resampleRule = formData.resample_rule;
   if (resampleMethod && resampleRule) {
+    const groupby_columns = ensureIsArray(queryObject.columns).map(column => {
+      if (isPhysicalColumn(column)) {
+        return column;
+      }
+      return column.label;
+    });

Review comment:
       There's already a `getColumnLabel` util for this:
   ```javascript
   const groupby_columns = ensureIsArray(queryObject.columns).map(column => getColumnLabel);
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org