You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "syedbarimanjan (via GitHub)" <gi...@apache.org> on 2025/05/05 16:13:57 UTC
[PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
syedbarimanjan opened a new pull request, #33364:
URL: https://github.com/apache/superset/pull/33364
### SUMMARY
Filters werent applying to multiple layers deck.gl chart because they werent being passed down to the sublayers properly. Fixes #13731
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #33364:
URL: https://github.com/apache/superset/pull/33364#issuecomment-2867361203
Approving CI to run again 🤞
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "syedbarimanjan (via GitHub)" <gi...@apache.org>.
syedbarimanjan commented on code in PR #33364:
URL: https://github.com/apache/superset/pull/33364#discussion_r2080289533
##########
superset/viz.py:
##########
@@ -1532,7 +1532,31 @@ def get_data(self, df: pd.DataFrame) -> VizData:
slice_ids = self.form_data.get("deck_slices")
slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all()
+
+ features: dict[str, list[Any]] = {}
Review Comment:
@mistercrunch Thanks for the suggestion. I looked into it, and it seems that this is not possible to be done in this pr as it would need a large refactor of the deckgl plugin.
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch closed pull request #33364: fix(deckgl): fix deckgl multiple layers chart filter and viewport
URL: https://github.com/apache/superset/pull/33364
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "syedbarimanjan (via GitHub)" <gi...@apache.org>.
syedbarimanjan commented on code in PR #33364:
URL: https://github.com/apache/superset/pull/33364#discussion_r2080289533
##########
superset/viz.py:
##########
@@ -1532,7 +1532,31 @@ def get_data(self, df: pd.DataFrame) -> VizData:
slice_ids = self.form_data.get("deck_slices")
slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all()
+
+ features: dict[str, list[Any]] = {}
Review Comment:
Thanks for the suggestion. I looked into it, and it seems that this is not possible to be done in this pr as it would need a large refactor of the deckgl plugin.
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "syedbarimanjan (via GitHub)" <gi...@apache.org>.
syedbarimanjan commented on code in PR #33364:
URL: https://github.com/apache/superset/pull/33364#discussion_r2080289533
##########
superset/viz.py:
##########
@@ -1532,7 +1532,31 @@ def get_data(self, df: pd.DataFrame) -> VizData:
slice_ids = self.form_data.get("deck_slices")
slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all()
+
+ features: dict[str, list[Any]] = {}
Review Comment:
@mistercrunch Thanks for the suggestion. I looked into it, and it seems that this is not possible to be done in this pr.
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #33364:
URL: https://github.com/apache/superset/pull/33364#issuecomment-2852488413
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/33364?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `16 lines` in your changes missing coverage. Please review.
> Project coverage is 65.69%. Comparing base [(`76d897e`)](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`3744dcb`)](https://app.codecov.io/gh/apache/superset/commit/3744dcb1c4c5001a1c85a1e3f47dc8a56860ae37?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1828 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/33364?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/viz.py](https://app.codecov.io/gh/apache/superset/pull/33364?src=pr&el=tree&filepath=superset%2Fviz.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdml6LnB5) | 0.00% | [16 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/33364?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #33364 +/- ##
==========================================
+ Coverage 60.48% 65.69% +5.20%
==========================================
Files 1931 553 -1378
Lines 76236 39982 -36254
Branches 8568 0 -8568
==========================================
- Hits 46114 26267 -19847
+ Misses 28017 13715 -14302
+ Partials 2105 0 -2105
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/33364/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/33364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.23% <0.00%> (-0.92%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/33364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/33364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.69% <0.00%> (-1.12%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/33364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.69% <0.00%> (+2.19%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/33364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.27% <0.00%> (+3.64%)` | :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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/33364?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33364:
URL: https://github.com/apache/superset/pull/33364#issuecomment-2860363765
@rusackas Ephemeral environment spinning up at http://34.208.227.125:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #33364:
URL: https://github.com/apache/superset/pull/33364#discussion_r2078587538
##########
superset/viz.py:
##########
@@ -1532,7 +1532,31 @@ def get_data(self, df: pd.DataFrame) -> VizData:
slice_ids = self.form_data.get("deck_slices")
slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all()
+
+ features: dict[str, list[Any]] = {}
Review Comment:
Given this, it might inform doing this in JS if that's possible, and possibly hitting `/api/chart/data` N times (for each chart), and joining the data client-side.
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #33364:
URL: https://github.com/apache/superset/pull/33364#discussion_r2073755750
##########
superset/viz.py:
##########
@@ -1532,7 +1532,31 @@ def get_data(self, df: pd.DataFrame) -> VizData:
slice_ids = self.form_data.get("deck_slices")
slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all()
+
+ features: dict[str, list[Any]] = {}
+
+ for slc in slices:
+ form_data = slc.form_data
+
+ form_data["extra_filters"] = self.form_data.get("extra_filters", [])
+ form_data["extra_form_data"] = self.form_data.get("extra_form_data", {})
+ form_data["adhoc_filters"] = self.form_data.get("adhoc_filters")
Review Comment:
### Shared Filter Object Reference Across Slices <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The filters from the parent visualization are directly assigned to each slice's form_data without creating a deep copy, which could lead to unintended modifications of shared filter objects across slices.
###### Why this matters
If multiple slices process these filters simultaneously, they could interfere with each other's filter state since they're referencing the same object in memory.
###### Suggested change ∙ *Feature Preview*
Create deep copies of the filter objects before assigning them to each slice's form_data:
```python
for slc in slices:
form_data = slc.form_data
form_data["extra_filters"] = copy.deepcopy(self.form_data.get("extra_filters", []))
form_data["extra_form_data"] = copy.deepcopy(self.form_data.get("extra_form_data", {}))
form_data["adhoc_filters"] = copy.deepcopy(self.form_data.get("adhoc_filters"))
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8db33baf-4361-4c16-8fd9-462ea2c96b7f/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8db33baf-4361-4c16-8fd9-462ea2c96b7f?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8db33baf-4361-4c16-8fd9-462ea2c96b7f?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8db33baf-4361-4c16-8fd9-462ea2c96b7f?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8db33baf-4361-4c16-8fd9-462ea2c96b7f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:f96bc113-cdb3-4d82-90e0-5e585e674a47 -->
[](f96bc113-cdb3-4d82-90e0-5e585e674a47)
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "syedbarimanjan (via GitHub)" <gi...@apache.org>.
syedbarimanjan commented on PR #33364:
URL: https://github.com/apache/superset/pull/33364#issuecomment-2871916778
@rusackas
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33364:
URL: https://github.com/apache/superset/pull/33364#issuecomment-2860337652
@rusackas Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/14893110370). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #33364:
URL: https://github.com/apache/superset/pull/33364#discussion_r2078586348
##########
superset/viz.py:
##########
@@ -1532,7 +1532,31 @@ def get_data(self, df: pd.DataFrame) -> VizData:
slice_ids = self.form_data.get("deck_slices")
slices = db.session.query(Slice).filter(Slice.id.in_(slice_ids)).all()
+
+ features: dict[str, list[Any]] = {}
Review Comment:
noting that we're trying to deprecate `viz.py` in favor of `/api/v1/chart/data`, though might be out-of-scope for this PR. I think some visualization like the deck.gl ones will need to migrate to the chart/data endpoint eventually. Pilling on more stuff onto viz.py might make it harder to migrate.
--
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
Re: [PR] fix(deckgl): fix deckgl multiple layers chart filter and viewport [superset]
Posted by "syedbarimanjan (via GitHub)" <gi...@apache.org>.
syedbarimanjan commented on PR #33364:
URL: https://github.com/apache/superset/pull/33364#issuecomment-2858835815
@geido
--
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