amaannawab923 commented on PR #33559: URL: https://github.com/apache/superset/pull/33559#issuecomment-2910208898
> I didn't follow the discussion too closely here, but shouldn't we keep the control for changing the scales? Also, we should make sure that the existing charts are not affected, and the best way to do it would be to either set the control to false by default, or if we need it to be enabled by default, then we should create a migration for existing Radar charts. Also, if there's a consensus about changing the name from "Normalise" to something else, could you change the variable and functions names in the code as well? Hi @kgabryje , as per discussion with @betodealmeida @geido , Since the radar chart was broken , we have agreed not to keep backwards compatibility & no toggle or control will be available hence the pr got changed from feature to fix since this is a fix for radar chart Also for the normalisation naming , it was a visiblity issue , that whether if we show the end user normalisation or not.... But on an implementation level , we are normalising the variables on scale of 0 to 1 with respect to max metric value hence renaming wont be required Thanks for the comment , i will update these details as well in pr description -- 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]
