You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/25 14:31:36 UTC

[GitHub] [incubator-superset] zhaoyongjie opened a new pull request #11060: remove useless filters in form_data

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


   ### SUMMARY
   After the filter transformation is done(extra_filters into adhoc_filters; split adhoc_filter into base filters)
   
   adhoc_filters and extra_filters are useless. remote it,  easy to debug and understand the filter logic


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

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



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

[GitHub] [incubator-superset] villebro commented on a change in pull request #11060: refactor: remove useless filters in form_data

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



##########
File path: superset/viz.py
##########
@@ -341,6 +341,9 @@ def process_query_filters(self) -> None:
         utils.convert_legacy_filters_into_adhoc(self.form_data)
         merge_extra_filters(self.form_data)
         utils.split_adhoc_filters_into_base_filters(self.form_data)
+        # Normalize various filters, and then remove the original filters
+        self.form_data.pop("adhoc_filters", None)
+        self.form_data.pop("extra_filters", None)

Review comment:
       As we're mutating `self.form_data` in the above function calls, I wonder if this logic should be moved there?




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

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



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

[GitHub] [incubator-superset] zhaoyongjie commented on a change in pull request #11060: refactor: remove useless filters in form_data

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



##########
File path: superset/viz.py
##########
@@ -341,6 +341,9 @@ def process_query_filters(self) -> None:
         utils.convert_legacy_filters_into_adhoc(self.form_data)
         merge_extra_filters(self.form_data)
         utils.split_adhoc_filters_into_base_filters(self.form_data)
+        # Normalize various filters, and then remove the original filters
+        self.form_data.pop("adhoc_filters", None)
+        self.form_data.pop("extra_filters", None)

Review comment:
       Is the logic of changing the filter here?




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

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



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

[GitHub] [incubator-superset] zhaoyongjie commented on a change in pull request #11060: refactor: remove useless filters in form_data

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



##########
File path: superset/viz.py
##########
@@ -341,6 +341,9 @@ def process_query_filters(self) -> None:
         utils.convert_legacy_filters_into_adhoc(self.form_data)
         merge_extra_filters(self.form_data)
         utils.split_adhoc_filters_into_base_filters(self.form_data)
+        # Normalize various filters, and then remove the original filters
+        self.form_data.pop("adhoc_filters", None)
+        self.form_data.pop("extra_filters", None)

Review comment:
       I want to put all of modifying the filter together




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

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



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

[GitHub] [incubator-superset] zhaoyongjie closed pull request #11060: refactor: remove useless filters in form_data

Posted by GitBox <gi...@apache.org>.
zhaoyongjie closed pull request #11060:
URL: https://github.com/apache/incubator-superset/pull/11060


   


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

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



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

[GitHub] [incubator-superset] zhaoyongjie commented on a change in pull request #11060: refactor: remove useless filters in form_data

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



##########
File path: superset/viz.py
##########
@@ -341,6 +341,9 @@ def process_query_filters(self) -> None:
         utils.convert_legacy_filters_into_adhoc(self.form_data)
         merge_extra_filters(self.form_data)
         utils.split_adhoc_filters_into_base_filters(self.form_data)
+        # Normalize various filters, and then remove the original filters
+        self.form_data.pop("adhoc_filters", None)
+        self.form_data.pop("extra_filters", None)

Review comment:
       Is the logic of changing the filter here?




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

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



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

[GitHub] [incubator-superset] codecov-commenter commented on pull request #11060: refactor: remove useless filters in form_data

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


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11060?src=pr&el=h1) Report
   > Merging [#11060](https://codecov.io/gh/apache/incubator-superset/pull/11060?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `4.40%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11060/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11060?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11060      +/-   ##
   ==========================================
   - Coverage   65.79%   61.38%   -4.41%     
   ==========================================
     Files         816      383     -433     
     Lines       38422    24188   -14234     
     Branches     3621        0    -3621     
   ==========================================
   - Hits        25280    14848   -10432     
   + Misses      13034     9340    -3694     
   + Partials      108        0     -108     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #cypress | `?` | |
   | #javascript | `?` | |
   | #python | `61.38% <100.00%> (-0.03%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11060?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/viz.py](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `57.49% <100.00%> (+0.05%)` | :arrow_up: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | [superset/utils/celery.py](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `82.14% <0.00%> (-3.58%)` | :arrow_down: |
   | [superset/result\_set.py](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.69% <0.00%> (-1.66%)` | :arrow_down: |
   | [...c/components/ListViewCard/ListViewCard.stories.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL0xpc3RWaWV3Q2FyZC5zdG9yaWVzLnRzeA==) | | |
   | [...c/dashboard/components/gridComponents/Markdown.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL01hcmtkb3duLmpzeA==) | | |
   | [...set-frontend/src/dashboard/util/getLocationHash.ts](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldExvY2F0aW9uSGFzaC50cw==) | | |
   | [...set-frontend/src/dashboard/reducers/datasources.js](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy5qcw==) | | |
   | [superset-frontend/src/explore/AdhocFilter.js](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQWRob2NGaWx0ZXIuanM=) | | |
   | [...-frontend/src/components/IndeterminateCheckbox.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSW5kZXRlcm1pbmF0ZUNoZWNrYm94LnRzeA==) | | |
   | ... and [427 more](https://codecov.io/gh/apache/incubator-superset/pull/11060/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11060?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11060?src=pr&el=footer). Last update [d056e3d...b1297a9](https://codecov.io/gh/apache/incubator-superset/pull/11060?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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



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