namannitr commented on PR #16903: URL: https://github.com/apache/echarts/pull/16903#issuecomment-5360496972
Thanks for landing the aggregate/id migration design here. Two correctness items from the experimental plugin that are worth checking against this branch before it leaves draft (they are easy to miss because `src` and `dist` diverged): - `SUM` must **not** initialize to `0`. That skips the first row of every group and zeroes singleton groups. Plugin history: 100pah/echarts-simple-transform#1 / #3; rebuilt bundle: 100pah/echarts-simple-transform#9. - Arithmetic methods should coerce cells (`Number` / dataset parsers). `'400' + '100'` string concat is a common `objectRows` footgun (100pah/echarts-simple-transform#6). Also, this PR’s `aggregate` still groups by **one** dimension. Long-format “one series per value of column X” is a reshape, not an aggregation — opened #21742 so that discussion does not block this migration. I am not proposing extra API on this PR; just flagging the SUM seed/`dist` trap so core does not re-ship it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
