You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Mattc1221 (via GitHub)" <gi...@apache.org> on 2023/04/02 02:52:24 UTC
[GitHub] [superset] Mattc1221 opened a new pull request, #23551: [WIP]feat: Add deck.gl Heatmap Visualization
Mattc1221 opened a new pull request, #23551:
URL: https://github.com/apache/superset/pull/23551
<!---
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 -->
Adds the heatmap layer from deck.gl [here](https://deck.gl/docs/api-reference/aggregation-layers/heatmap-layer).
**_Rationale_**:
Superset already has supported functionality for other deck.gl visualization layers (e.g. Arc, Geojson, Grid, Hex, Path, Polygon, Scatter, Screengrid). Since Superset already contains the logic to handle deck.gl layers, it is straightforward and easy to maintain to add new deck.gl layers.
By expanding the range of available deck.gl visualization layers, users will have more options to choose from when creating their visualizations. This will allow them to tailor their visualizations to their specific needs and explore their data in different ways. It will also allow Superset to better compete with other data visualization tools on the market that offer a wider range of visualization options.
Overall, I think adding more deck.gl layers to Superset is a great idea and has the potential to significantly improve the user experience and the insights that users can gain from their data.
**_Design Decisions_**
The addition of the Heatmap layer follows the structure of existing deck.gl layers to maintain readability and consistency within the codebase.
Structure:
The directory `superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Heatmap/` contains the front end structure for the heat map
- `controlPanel` configures the form to grab relevant information specified by the user to create and tune the heatmap.
- `Heatmap.jsx` contains functions that define the Heatmap Layer, control the tooltip when hover the chart, and to get relevant data
- `index.js` serves as the wrapper for the heatmap component and provides a metadata function to grab information to descrive what this visualization is/does
The Back End code for this feature follows the back end code for other deck.gl visualizations as well, providing functions to structure queried data for the Heatmap component.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
New visualization recording:
https://user-images.githubusercontent.com/36670322/229327601-22f81978-174e-41c0-8f8f-6cf7ea66691d.mov
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. Start Superset
2. Click on the `Charts` tab at the top of the screen
3. Click the `+ Chart` button at the top right
4. Load a dataset with geo-positional data (longitude/latitude)
5. Search the charts for `Deck.gl Heatmap` or click on the `Map` chart category
6. Select `deck.gl Heatmap` and hit `Create New Chart`
7. Update the `Longitude & Latitude` field to the correct columns
8. Select a `Weight` function
9. Select `Create Chart`
10. There should now be a heat map layer present on the screen
- Note: the underlying map component may not be visible if the MAP_BOX_API_KEY is not set. To set it follow the instructions [here](https://superset.apache.org/docs/frequently-asked-questions/#why-is-the-map-not-visible-in-the-geospatial-visualization)
11. Change the options in the control panel, check that the information tooltips are correct/make sense for each field
12. Check that when hovering a point on the map, the latitude/longitude shows up correctly
### 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
- [X] 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] lolvib commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "lolvib (via GitHub)" <gi...@apache.org>.
lolvib commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1560260895
>
Appreciate it @Mattc1221! Great work 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.
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] Mattc1221 commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "Mattc1221 (via GitHub)" <gi...@apache.org>.
Mattc1221 commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1514164884
@rusackas Thanks for re-running the tests :)
The failing tests should now be fixed!
--
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] rusackas commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1511524454
Running the tests... thanks for this PR! I hope to review/test as well in a day or three :)
--
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 diff in pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #23551:
URL: https://github.com/apache/superset/pull/23551#discussion_r1170947949
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Heatmap/Heatmap.jsx:
##########
@@ -0,0 +1,82 @@
+/**
Review Comment:
Could you please refactor this file to typescript? I know that the rest of deckgl plugins are written in javascript, but our policy is for create the new files in typescript and SOME day refactor the old js files to ts too 🙂 (correct me if I'm wrong @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
[GitHub] [superset] kgabryje commented on a diff in pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #23551:
URL: https://github.com/apache/superset/pull/23551#discussion_r1170947949
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Heatmap/Heatmap.jsx:
##########
@@ -0,0 +1,82 @@
+/**
Review Comment:
Could you please refactor this file to typescript? I know that the rest of deckgl plugins are written in javascript, but our policy is to create the new files in typescript and SOME day refactor the old js files to ts too 🙂 (correct me if I'm wrong @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
[GitHub] [superset] Mattc1221 commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "Mattc1221 (via GitHub)" <gi...@apache.org>.
Mattc1221 commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1555975847
Hey @kgabryje! could we run the CI tests again? With the latest changes, the tests pass locally and also on my GitHub actions page!
<img width="1335" alt="Screenshot 2023-05-20 at 2 18 16 PM" src="https://github.com/apache/superset/assets/36670322/746e1e8a-abdc-4d71-9511-181dd3249d4d">
--
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] Mattc1221 commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "Mattc1221 (via GitHub)" <gi...@apache.org>.
Mattc1221 commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1558561362
Big thanks to @kgabryje @villebro and @v1bg for reviewing this PR!
> Hi! This is really cool. Just deployed out and tested and everything looks good with the new heatmap chart type, but just FYI when you try to add the heatmap to the "deck.gl Multiple Layers" chart type it doesn't appear to be working. Wanted to mention this to see if it's a known issue.
This may have been an oversight and may be a missing import in the Multiple Layers chart type. I'll look into this further later this week and open a corresponding issue!
--
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 #23551: feat: Add deck.gl Heatmap Visualization
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1511539603
## [Codecov](https://codecov.io/gh/apache/superset/pull/23551?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 [#23551](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f36caf3) into [master](https://codecov.io/gh/apache/superset/commit/9920ab3fd9230674e159b2e649565d4a6dc1db88?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9920ab3) will **decrease** coverage by `10.86%`.
> The diff coverage is `67.65%`.
> :exclamation: Current head f36caf3 differs from pull request most recent head 882e902. Consider uploading reports for the commit 882e902 to get more accurate results
```diff
@@ Coverage Diff @@
## master #23551 +/- ##
===========================================
- Coverage 67.53% 56.67% -10.86%
===========================================
Files 1907 1922 +15
Lines 73473 74060 +587
Branches 7976 8104 +128
===========================================
- Hits 49617 41972 -7645
- Misses 21807 30016 +8209
- Partials 2049 2072 +23
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.09% <ø> (+0.38%)` | :arrow_up: |
| python | `59.41% <ø> (-22.87%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `53.02% <ø> (+0.55%)` | :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/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...t-ui-chart-controls/src/shared-controls/mixins.tsx](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9taXhpbnMudHN4) | `16.66% <ø> (ø)` | |
| [...d/packages/superset-ui-chart-controls/src/types.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3R5cGVzLnRz) | `100.00% <ø> (ø)` | |
| [.../packages/superset-ui-core/src/chart/types/Base.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY2hhcnQvdHlwZXMvQmFzZS50cw==) | `100.00% <ø> (ø)` | |
| [...i-core/src/color/colorSchemes/sequential/common.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY29sb3IvY29sb3JTY2hlbWVzL3NlcXVlbnRpYWwvY29tbW9uLnRz) | `100.00% <ø> (ø)` | |
| [...s/superset-ui-core/src/components/SafeMarkdown.tsx](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvY29tcG9uZW50cy9TYWZlTWFya2Rvd24udHN4) | `66.66% <ø> (ø)` | |
| [...-core/src/hooks/useChangeEffect/useChangeEffect.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvaG9va3MvdXNlQ2hhbmdlRWZmZWN0L3VzZUNoYW5nZUVmZmVjdC50cw==) | `100.00% <ø> (ø)` | |
| [...hooks/useComponentDidMount/useComponentDidMount.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvaG9va3MvdXNlQ29tcG9uZW50RGlkTW91bnQvdXNlQ29tcG9uZW50RGlkTW91bnQudHM=) | `100.00% <ø> (ø)` | |
| [...oks/useComponentDidUpdate/useComponentDidUpdate.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvaG9va3MvdXNlQ29tcG9uZW50RGlkVXBkYXRlL3VzZUNvbXBvbmVudERpZFVwZGF0ZS50cw==) | `100.00% <ø> (ø)` | |
| [...src/hooks/useElementOnScreen/useElementOnScreen.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvaG9va3MvdXNlRWxlbWVudE9uU2NyZWVuL3VzZUVsZW1lbnRPblNjcmVlbi50cw==) | `100.00% <ø> (ø)` | |
| [...erset-ui-core/src/hooks/usePrevious/usePrevious.ts](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvaG9va3MvdXNlUHJldmlvdXMvdXNlUHJldmlvdXMudHM=) | `100.00% <ø> (ø)` | |
| ... and [150 more](https://codecov.io/gh/apache/superset/pull/23551?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
... and [491 files with indirect coverage changes](https://codecov.io/gh/apache/superset/pull/23551/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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 merged pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro merged PR #23551:
URL: https://github.com/apache/superset/pull/23551
--
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] Mattc1221 commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "Mattc1221 (via GitHub)" <gi...@apache.org>.
Mattc1221 commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1528891241
@kgabryje I really appreciate your first review! The changes requests should now be addressed and I would love to get another round of review!
--
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 pull request #23551: feat: [WIP] Add deck.gl Heatmap Visualization
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1495443146
This is really exciting work @Mattc1221 ! I'm marking myself as a reviewer and will do my best to review+test this later this week.
--
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] Mattc1221 commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "Mattc1221 (via GitHub)" <gi...@apache.org>.
Mattc1221 commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1510469902
Hi @villebro I believe this PR should now be ready for review!
However, I noticed the tests have not run and I was wondering if I needed to close the PR and open a new one?
--
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] v1bg commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "v1bg (via GitHub)" <gi...@apache.org>.
v1bg commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1557636186
Hi! This is really cool. Just deployed out and tested and everything looks good with the new heatmap chart type, but just FYI when you try to add the heatmap to the "deck.gl Multiple Layers" chart type it doesn't appear to be working. Wanted to mention this to see if it's a known issue.
Thanks 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
[GitHub] [superset] Mattc1221 commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "Mattc1221 (via GitHub)" <gi...@apache.org>.
Mattc1221 commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1514290515
Thanks for the review @kgabryje 😀I'll update those JavaScript files to typescript sometime tomorrow evening!
--
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 diff in pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #23551:
URL: https://github.com/apache/superset/pull/23551#discussion_r1170950562
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Heatmap/index.js:
##########
@@ -0,0 +1,45 @@
+/**
Review Comment:
Same as above - js -> ts 🙏
--
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 pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1534153403
@Mattc1221 looks like there are some linting issues
--
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] v1bg commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "v1bg (via GitHub)" <gi...@apache.org>.
v1bg commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1560261537
Awesome, appreciate it @Mattc1221! Great work 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.
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 pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1556112063
@Mattc1221 I started CI and will review today 👍
--
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 pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1534137277
@Mattc1221 Sorry for the long delay and thank you for implementing the changes 🙂 Will merge as soon as the CI passes
--
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] Mattc1221 commented on pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "Mattc1221 (via GitHub)" <gi...@apache.org>.
Mattc1221 commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1547065617
@kgabryje Thanks again for the review, and I apologize for the delayed response! I've rebased this branch to include the latest changes on the master branch and fixed the linting errors for the failing file `plugins/legacy-preset-chart-deckgl/types/external.d.ts`!
--
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 pull request #23551: feat: Add deck.gl Heatmap Visualization
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23551:
URL: https://github.com/apache/superset/pull/23551#issuecomment-1514287933
Awesome first contribution, congrats! I left a comment about refactoring js to ts, but otherwise the code looks great 🎉
--
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