superset icon indicating copy to clipboard operation
superset copied to clipboard

[SIP-120] Enhanced Time Comparisons on Bar and Line Charts

Open yousoph opened this issue 1 year ago • 6 comments

Motivation

  • Allow for more customization on bar and line charts when using the time comparison feature
  • Increase the flexibility of selecting a comparison date

Proposed Changes

Add the ability to do the following in Bar charts when using the Time Shift feature.

Time Shift Selection:

  • Select a custom time shift by selecting a start date from a date selector. This would allow for simpler comparison when looking at a holiday across multiple years. Black Friday in 2023 is on a different calendar date than Black Friday 2022 and isn’t currently easy to compare using the Time Shift.
  • Ability to easily shift the time based on the length of the original time range. For example, if the original chart time filter was 3 weeks and 1 day, the user could easily shift the time range to look at the 3 week and 1 day long period directly preceding the existing chart filter.
  • Display the comparison ranges in the control panel for the user to easily verify the comparison dates

Chart customizations:

  • Customize whether the comparison bar should appear to the left, right, or in chronological order vs the original bar
  • Customize the tooltip to include additional information such as the difference between the comparison period and the original or the % change between the two time periods
  • Display the comparison and original time ranges in the chart area for non-creator chart users to easily understand the comparison dates

For Line Charts, we’d like to make all the above changes with the same patterns (other than customizing where the comparison bar appears).

New or Changed Public Interfaces

See UI mockup here: bar_comparison

New dependencies

None

Migration Plan and Compatibility

In moving the controls to a new section in the Explore Control Panel, we’ll need to ensure that existing controls on previously created charts are retained.

Rejected Alternatives

We considered whether these changes could require a new chart type or a new control for the time shift, but they had enough overlap with the current Time Shift functionality that it made the most sense to integrate it with existing charts and controls.

yousoph avatar Mar 22 '24 06:03 yousoph

Thank you for the SIP @yousoph. Thinking about this SIP and https://github.com/apache/superset/issues/27432, and considering this comment, it seems we are dealing with 2 separate problems: 1 - How do we configure/support time shifts consistently between plugins (in or out of advanced analytics, how to configure/display multiple time shifts, API changes, etc) 2 - How do we represent time shifts for each chart type (extra columns in a table, lines/bars with different patterns, additional and exclusive controls per visualization such as bar placement).

I would suggest discussing point number 1 in a specific SIP to make sure we have a consistent behavior between plugins and answer the following:

In all other chart types, time shifts are inside the Advanced Analytics panel. Here we are proposing to show them in a different section. I'm worried this will decrease UI consistency, as users frequently associate available features with the categories that the panel represent. In other words, once they associate that time shifts are inside the Advanced Analytics, they will always expect to find that feature in the same place between plugins.

I understand that the current implementation of time shifts is limited and not flexible as the one we are proposing here. Assuming that all plugins will adhere to this new form of configuring a time shift, and trying to avoid the situation where time shifts behave differently between plugins, would be possible to change this globally? Apart from UI consistency, I'm also thinking about the required API/query object changes and maintaining a single implementation. If we want to do this in multiple steps, I would suggest to implement the new table features using the current time shift implementation and later work on the time shift changes globally.

Could you expand on what are the required changes for our query API/concepts? I think this is the main point of the SIP given the many discussions happening on https://github.com/apache/superset/pull/27355 and https://github.com/apache/superset/pull/27386 about potential solutions.

Then, after resolving point 1, this SIP and https://github.com/apache/superset/issues/27432 would deal with point number 2.

Discussing point number 1 in a split way, introduces many inconsistencies such as:

  • This SIP proposes time shifts to be configured in its own collapse panel
  • https://github.com/apache/superset/issues/27432 proposes a different approach using a checkbox called Enable time comparison
  • The rest of the plugins propose to configure time shifts using Advanced Analytics

By the way, I like the idea of Time comparison being a separate panel. We could create a component for that and apply to all plugins.

michael-s-molina avatar Mar 22 '24 13:03 michael-s-molina

I think if we create a new Time Comparison panel, that we should do that for any charts that currently have time comparison in Advanced Analytics.

yousoph avatar Mar 22 '24 21:03 yousoph

@yousoph The Time Comparison feature is not only about "comparing different date filters," but it also involves:

  1. Calculating ratios, differences, or percentage changes.
  2. Rolling data based on the date-time index.
  3. Filling up lost values

IMO, we should consider "how to improve UI/UX in advanced analytics" without re-implementing parts of the previous function in different charts.

zhaoyongjie avatar Mar 26 '24 23:03 zhaoyongjie

That's a great point @zhaoyongjie. Some of the features you mentioned are very important when comparing time periods. @zhaoyongjie @yousoph @eschutho @kasiazjc Do you think a working session (2 hours) would be interesting to discuss possible solutions?

michael-s-molina avatar Mar 27 '24 11:03 michael-s-molina

+1 on bifurcating the discussion into two topics: configuration and display. It would be ideal to have the controls/config be a consistent interface between all plugins that use this feature. We should strive to make the plugin control panels as cohesive/consistent as possible, as this has historically been painful to improve or maintain.

rusackas avatar Apr 02 '24 16:04 rusackas

@yousoph should we kick off a [VOTE] and make it official?

rusackas avatar May 07 '24 16:05 rusackas

Closing as voted down, but only in order to split it into two SIPs as described above.

I don't think we need as much discussion on #1 any more since the checkbox idea is out of the running, but it'll be good to have official consensus on the proposal and migration plan nonetheless.

rusackas avatar Jul 02 '24 15:07 rusackas