-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Displayed as a value multiplied by 100 before displaying the percentage #6586
Comments
That percentage multiplier thing sounds familiar, as in... I think @eradman was doing some work to fix something else about a month ago, and this might be an unintended side effect. @eradman If this doesn't immediately jump out at you, I can go digging through stuff and try and turn up the thing I was thinking of. 😄 |
It sounds like the result of #6344. I will try to confirm Both and and new libraries appear to produce the same results:
|
Ahhhh. Yep, I think that's the one I was half-remembering. 😄 Thanks for confirming I wasn't imagining things @eradman. 😁 |
@eradman @justinclift Thanks! Understood. I agree with you. I also think it would be natural to display the percentage as 0.25 => 25%. I think the update makes sense. The next issue is compatibility. This will be a breaking change. This change will require Redash users to rewrite their queries. So let me suggest one thing. Can we do it like below?
What do you think about this? |
That's a good point about the backward compatibility aspect. If we keep the same default as the previous versions, that'll likely be one less thing people (who are upgrading) have to worry about. |
@masayuki038 the strategy you outlined make sense. I will look into adding an environment variable for maintaining compatibility. |
@justinclift @eradman Thanks for your confirmation.
Thanks! |
This plan makes sense to me! Thank you all! |
Hi Everyone, Is there any movement on this issue? Right now our existing alerts/queries/dashboards are still off by 2 orders of magnitude. If no movement, is there anything I can do in the meantime? Thanks! |
We have a PR open here, but we're waiting to see if numbro updates on their end. If you need a fix right now, I'd suggest it's probably best to update your queries to divide by 100, or change the formatting on the outputs - but this is obviously pretty time intensive. You could also build the image in #6595 yourself and deploy that instead, maybe? |
@willvictor #6595 was merged. Please try it if you have time. Thank you for the report. |
Issue Summary
@willvictor reported in #6579:
I did notice what seems to be a breaking change in the latest version:
SELECT 100 * 6/10 as perc
, then the format of 0,0.0% would display 60.0%, but now it displays 6000.0%.Do you know what might have caused this? Was it intentional?
Steps to Reproduce
(T.B.D.)
Technical details:
The text was updated successfully, but these errors were encountered: