fix: Time shifts calculation for ECharts plugins

[michael-s-molina]

SUMMARY

As part of of [SIP-50] Proposal for using ECharts as our main charting library, we're currently testing Line and Area chart migrations using Airbnb's production data. During these tests, we found many problems when comparing how legacy and ECharts plugins calculate and display time shifts. This PR fixes the following issues:

Time shifts when the their granularity do not match the chart's granularity

In this example, we have a chart with a monthly granularity but with a 52 weeks offset. This is how the legacy line chart correctly renders the chart:


The ECharts line chart was incorrectly calculating the time shift when applying the 52 weeks offset due to differences in terms of granularity. Observe how the peak in November 2003 (296) does not match the time shift in 2004 (70).


Now the ECharts line chart correctly calculates the time shift and matches its legacy version.

How time shifts are displayed when there's missing data

Previously, when time shift data was not available, the chart was rendered with line gaps which is different than the legacy version.


Now, the line gaps are gone and the dots are connected without a data point which is exactly how the legacy chart displays this scenario.


There is also an error about how the legacy version displays missing data depending on the chosen granularity. In this example we have a chart with daily granularity and we only have time shift data for some data points. The legacy chart displays a line for every data point and assumes that the time shift is the same as the series which is not correct.


With this fix, only available time shifts are displayed and the tooltip display the correct information when a time shift is not available.

How multiple time shifts are displayed

Previously, multiple time shifts were displayed using the same line pattern, which is confusing and require users to check the tooltips to see the actual time shift.


Now, it behaves exactly as the legacy version where each time shift is displayed using a different line pattern.

TESTING INSTRUCTIONS

To test the PR use the Advanced Analytics controls and play with different time shift settings. I tested the algorithm using Airbnb's production data using more than 100 charts and added extra unit tests.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Attention: Patch coverage is 86.27451% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.18%. Comparing base (76d897e) to head (48f467f).
Report is 1094 commits behind head on master.

Files with missing lines	Patch %	Lines
...gin-chart-echarts/src/Timeseries/transformProps.ts	33.33%	5 Missing and 1 partial ⚠️
superset/common/query_context_processor.py	97.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #28432      +/-   ##
==========================================
+ Coverage   60.48%   70.18%   +9.69%     
==========================================
  Files        1931     1944      +13     
  Lines       76236    77322    +1086     
  Branches     8568     8672     +104     
==========================================
+ Hits        46114    54265    +8151     
+ Misses      28017    20933    -7084     
- Partials     2105     2124      +19     

Flag	Coverage Δ
hive	49.09% <10.52%> (-0.08%)	⬇️
javascript	57.73% <53.84%> (+0.02%)	⬆️
mysql	77.13% <89.47%> (?)
postgres	77.24% <89.47%> (?)
presto	53.64% <10.52%> (-0.16%)	⬇️
python	83.41% <97.36%> (+19.92%)	⬆️
sqlite	76.69% <89.47%> (?)
unit	58.85% <78.94%> (+1.23%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[john-bodley]

Thanks @michael-s-molina for the fix. Overall this LGTM, especially given we synced up offline to discuss the underlying logic.

[Antonio-RiveroMartnez]

LGTM!

[eschutho]

lgtm

[github-actions]

Ephemeral environment shutdown and build artifacts deleted.