[GitHub] [incubator-superset] villebro commented on a change in pull request #8229: [POC] add echarts scatter
villebro commented on a change in pull request #8229: [POC] add echarts scatter URL: https://github.com/apache/incubator-superset/pull/8229#discussion_r326310227 ## File path: superset/viz.py ## @@ -2765,6 +2765,15 @@ def get_data(self, df): return self.nest_values(levels) +class EchartsScatter(BubbleViz): Review comment: It also doesn't have to be either done in the frontend or backend; can be both. Some postprocessing can be very expensive to move to the front, especially percentiles/quantiles and the like that require having a full distribution, so those would be preferable to perform in the backend, while others might be well suited for the frontend. So I think being able to offer at least some postprocessing options in the backend would be nice, leaving it up to the viz plugin developer to decide where to perform the calculations. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #8229: [POC] add echarts scatter
villebro commented on a change in pull request #8229: [POC] add echarts scatter URL: https://github.com/apache/incubator-superset/pull/8229#discussion_r324871580 ## File path: superset/viz.py ## @@ -2765,6 +2765,15 @@ def get_data(self, df): return self.nest_values(levels) +class EchartsScatter(BubbleViz): Review comment: Sounds reasonable. In all, I think this will be a big feature to implement, but can probably be dissected into smaller parts. I would probably start with implementing the pivot operation, as it is used quite frequently in viz's and should be easy to start with. I'm fairly overloaded with other stuff right now, but should be able to look into this in a few weeks time if nobody wants to take the lead. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #8229: [POC] add echarts scatter
villebro commented on a change in pull request #8229: [POC] add echarts scatter URL: https://github.com/apache/incubator-superset/pull/8229#discussion_r324804169 ## File path: superset/viz.py ## @@ -2765,6 +2765,15 @@ def get_data(self, df): return self.nest_values(levels) +class EchartsScatter(BubbleViz): Review comment: @kristw is there already a working plan for migrating some of the more complex legacy viz's to the new api, namely the ones that rely on Pandas functionality? Is the plan to do that logic in JS, och making the heavy lifting in the backend? I anticipate quite a bit of work there, and can lend a hand if necessary, especially if the new API needs new functionality.. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org