Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
yousoph opened this issue Mar 22, 2024 · 7 comments
Closed

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

yousoph opened this issue Mar 22, 2024 · 7 comments
Labels
sip Superset Improvement Proposal

Comments

@yousoph
Copy link
Member

yousoph commented Mar 22, 2024

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 yousoph added the sip Superset Improvement Proposal label Mar 22, 2024
@michael-s-molina
Copy link
Member

michael-s-molina commented Mar 22, 2024

Thank you for the SIP @yousoph. Thinking about this SIP and #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 #27355 and #27386 about potential solutions.

Then, after resolving point 1, this SIP and #27432 would deal with point number 2.

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

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.

@yousoph
Copy link
Member Author

yousoph commented Mar 22, 2024

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.

@zhaoyongjie
Copy link
Member

@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.

@michael-s-molina
Copy link
Member

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?

@rusackas
Copy link
Member

rusackas commented Apr 2, 2024

+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
Copy link
Member

rusackas commented May 7, 2024

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

@rusackas
Copy link
Member

rusackas commented Jul 2, 2024

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 rusackas closed this as completed Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sip Superset Improvement Proposal
Projects
Status: Denied / Closed / Discarded
Development

No branches or pull requests

4 participants