Skip to content

Fix a TypeError on complex number format with small enough number #3129

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

Conversation

ma-gnesium
Copy link

TypeError occurs with numbers that convert to scientific notation (ie small enough ones) when using number formats that require the complex algorithm of NumberFormatter.

This is:

- [x] a bugfix
- [ ] a new feature
- [ ] refactoring
- [ ] additional unit tests

Checklist:

Why this change is needed?

This fixes #3128

TypeError occurs with numbers that convert to scientific notation (ie small enough ones) when using number formats that require the complex algorithm of NumberFormatter.
@oleibman
Copy link
Collaborator

Your change works well for your use case. It does not seem to work well for large numbers, e.g. 1E20 (which are also not handled well by existing code). Might it be possible to to solve that problem as well, and add test cases for that and intermediate numbers (e.g. 1E5)?

@oleibman
Copy link
Collaborator

I've experimented a little with large numbers. number_format seems to return some flaky results. I think it would be fairly straightforward with the BCMath extension, but that is not a requirement at the moment.

@oleibman
Copy link
Collaborator

A function which might be usable can be found at https://stackoverflow.com/questions/66135441/converting-float-to-string-without-exponential-notation
You probably want to do something about trailing zeros.

@ma-gnesium
Copy link
Author

Thanks you for the feedback @oleibman.
Reconsidering the original bug report, I'm closing this PR because the effective fix from my point of vue is to use a correct format syntax, ie using comma as a thousands separator.

@ma-gnesium ma-gnesium closed this Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

TypeError : abs(): Argument #1 ($num) must be of type int|float, string given
2 participants