GabeLoins opened a new pull request #4663: [Request For Comments][Explore] Streamlined metric definitions for SQLA and Druid URL: https://github.com/apache/incubator-superset/pull/4663 This PR streamlines the definition of `AGGREGATE( column )`-style metrics in the explore view. It circumvents the fab view entirely for a much improved user experience. The metric is stored as a simple object of form {column, aggregate, label} in form data. When a query is run, the adhoc metric objects are sent along with saved metrics to the flask backend, where they are manually parsed in SQLA/Druid expressions. Here are some quick videos demoing what interactions look like: Adding and editing metrics:  Applying labels to metrics:  Guide to reviewers: - These streamlined metrics I refer to as `adhocMetrics`. Traditional metrics I refer to as `savedMetrics` - The bulk of the frontend logic lives in MetricsControl and AdhocMetricEditPopover. MetricsControl handles data massaging and the dropdown. AdhocMetricEditPopover handles most of the log for editing - The bulk of the flask logic is in connectors/druid/models.py and connectors/sqla/models.py. In those files I handle the parsing of adhoc metrics in sqla/druid aggregations. Open questions: - There is custom parsing logic all over viz.py. Since this PR changes the way we post form data, we may not be able to enable this feature on all viz types. I made a fix for line charts, but I fear there may be more such issues lurking. (See viz.py change for details) - How should we deal with displaying saved metrics in the dropdown? In this PR, the default order of options in the metrics dropdown is `[...columns, ...aggregates, ...saved metrics]`. I prefer this ordering because there are many saved metrics that this feature will replace. However I can see it being confusing initially to users to have this dropdown default to unexpected content. - Should we prevent aggregate/column combinations based on column type? Right now I just let the users do as they please. - How should we think about deprecating saved metrics that are just a column+aggregate? reviewers: @michellethomas @john-bodley @williaster @graceguo-supercat @mistercrunch (after putting up this PR I'll continue testing locally so there may be some bug fixes to come, but this is the done deal!)
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
