rusackas commented on PR #36214: URL: https://github.com/apache/superset/pull/36214#issuecomment-4753101232
Thanks for sticking with this @YousufFFFF. Couple of things before we can move on it. My bigger worry is the approach: sorting each `series.data` array independently doesn't reorder the category axis itself, and for a category axis ECharts aligns points by index across series. So I'm not sure per-series sorting actually fixes the skipped-category case from #35853 rather than just reshuffling within one series. Did you confirm the repro from the issue (the `202408` gap) renders correctly with multiple series? A screenshot of that exact dataset would help. Also the new code is `any` all the way through (`naturalCompare(a: any, b: any)`, the `(s as any).data` casts), which runs against our no-`any` efforts — can we type these properly? Lastly, the earlier copilot/bito notes about in-place mutation and the temporal-fallback mixing numbers/strings still look unaddressed in the diff. Thanks in advance for the touch-ups! -- 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]
