-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
fix: big number with trendline can't calculate cumsum #19542
Conversation
const TIME_GRAIN_MAP: Record<string, string> = { | ||
PT1S: 'S', | ||
PT1M: 'min', | ||
PT5M: '5min', | ||
PT10M: '10min', | ||
PT15M: '15min', | ||
PT30M: '30min', | ||
PT1H: 'H', | ||
P1D: 'D', | ||
P1M: 'MS', | ||
P3M: 'QS', | ||
P1Y: 'AS', | ||
// TODO: these need to be mapped carefully, as the first day of week | ||
// can vary from engine to engine | ||
// P1W: 'W', | ||
// '1969-12-28T00:00:00Z/P1W': 'W', | ||
// '1969-12-29T00:00:00Z/P1W': 'W', | ||
// 'P1W/1970-01-03T00:00:00Z': 'W', | ||
// 'P1W/1970-01-04T00:00:00Z': 'W', | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use explicit the Resample control to select.
Codecov Report
@@ Coverage Diff @@
## master #19542 +/- ##
=======================================
Coverage 66.59% 66.60%
=======================================
Files 1682 1682
Lines 64314 64316 +2
Branches 6559 6560 +1
=======================================
+ Hits 42832 42836 +4
+ Misses 19781 19779 -2
Partials 1701 1701
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Tested in local and LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the fix and the testing instructions, they really make validation so much easier 👍
(cherry picked from commit 2daa071)
🏷️ preset:2022.13 |
(cherry picked from commit 2daa071)
SUMMARY
This PR fixed that the Big Number with Trendline can't calculate with
cumsum
. In addition, I moved the Resample panel into the chart.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before
After
TESTING INSTRUCTIONS
cleaned_sales_data
dataset and theBig Number with Trendline
Charta.
order date
as the TIME COLUMN.b.
Month
as the TIME GRAIN.c.
COUNT(*)
as the METRIC.d.
cumsum
as the ROLLING FUNCTION.ADDITIONAL INFORMATION