Skip to content
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

Open
masayuki038 opened this issue Nov 9, 2023 · 12 comments
Open
Assignees

Comments

@masayuki038
Copy link
Collaborator

masayuki038 commented Nov 9, 2023

Issue Summary

@willvictor reported in #6579:


I did notice what seems to be a breaking change in the latest version:
Screen Shot 2023-11-08 at 9 00 11 AM

  • for table visualizations that have a percentage display, it seems the formatter now multiplies the value by 100 before displaying the percentage. Previously the formatter did not do this.
  • This means if I wanted to display 60% previously, I would write sql that would output 60%, for example 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:

  • Redash Version: redash/redash:preview
  • Browser/OS:
  • How did you install Redash:
@justinclift
Copy link
Member

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. 😄

@eradman
Copy link
Collaborator

eradman commented Nov 9, 2023

It sounds like the result of #6344. I will try to confirm

Both and and new libraries appear to produce the same results:

> var numbro = require("numbro");
> num = numbro(0.25);
t { _value: 0.25 }
> num.format('0.0%');
'25.0%'
> var numeral = require("numeral");
> num = numeral(0.25);
{ _input: 0.25, _value: 0.25 }
> num.format('0.0%');
'25.0%'

@justinclift
Copy link
Member

justinclift commented Nov 9, 2023

Ahhhh. Yep, I think that's the one I was half-remembering. 😄

Thanks for confirming I wasn't imagining things @eradman. 😁

@eradman
Copy link
Collaborator

eradman commented Nov 9, 2023

I can confirm that this behavior changed

Format:

value: 0.00
percent: 0.0%

Redash 23.11.0-dev
redash23-11-percent

Redash 10
redash10-percent

But I would say the new result is correct, since this is how numeral and numbro format a number when % is added!

@masayuki038
Copy link
Collaborator Author

@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?

  • Introducing a new environment variable that allows you to toggle whether to multiply by 100 or not
  • The next release will continue to display numbers that are not multiplied by 100 by default
    • Of course, it can toggle with a new environment variable
  • From the next major version up, the default will be to display the value multiplied by 100

What do you think about this?

@justinclift
Copy link
Member

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.

@eradman
Copy link
Collaborator

eradman commented Nov 10, 2023

@masayuki038 the strategy you outlined make sense. I will look into adding an environment variable for maintaining compatibility.

@masayuki038
Copy link
Collaborator Author

@justinclift @eradman Thanks for your confirmation.

@eradman

I will look into adding an environment variable for maintaining compatibility.

Thanks!

@willvictor
Copy link

This plan makes sense to me! Thank you all!

@willvictor
Copy link

willvictor commented Nov 20, 2023

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!
Will

@guidopetri
Copy link
Contributor

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?

@masayuki038
Copy link
Collaborator Author

@willvictor #6595 was merged.
I have confirmed "0.25" is displayed as "0.25%" with "0.00%" format in redash/redash:preview now.

image

Please try it if you have time. Thank you for the report.
If there are no particular problems, this issue will be closed after one week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To do
Development

No branches or pull requests

5 participants