Skip to content

Add support for string formatting of floats with ":f" format code #1651

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

Merged
merged 1 commit into from
Jan 1, 2020

Conversation

doyshinda
Copy link
Contributor

Also adds automatic conversion of int -> float when the ":f"
format code is specified.

@doyshinda
Copy link
Contributor Author

Contributes to #1613

@doyshinda doyshinda force-pushed the impl_format_for_float branch 2 times, most recently from 4bd71d7 to 64ff6e8 Compare December 28, 2019 22:06
@doyshinda doyshinda force-pushed the impl_format_for_float branch 2 times, most recently from f4334db to a54d015 Compare December 30, 2019 00:38
Copy link
Member

@coolreader18 coolreader18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, thanks for working on this!

let mut remaining: usize = magnitude_string.len();
for c in magnitude_string.chars() {

// Don't add separators to the floating decimal point of numbers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure exactly what the functionality of this is supposed to be, but you might want to do .splitn('.', 2) so that the splitting sort of has right associativity. Then you don't have to .collect() it and you can do let magnitude_integer_string = parts.next().unwrap(); and if let Some(part) = parts.next() { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to prevent adding an incorrect separator to floats, as this is what it looked like before:

>>>>> '{:,.2f}'.format(1234567890.1234)
'1,234,567,890,.12'

With this change, it's only adding the separator to the integer portion of the number:

>>>>> '{:,.2f}'.format(1234567890.1234)
'1,234,567,890.12'

I'll change it to the way you suggested, as that is cleaner.

Also adds automatic conversion of int -> float when the ":f"
format code is specified.

Fixes a f-string formatting style regression introduced in RustPython#1625.
@doyshinda doyshinda force-pushed the impl_format_for_float branch from a54d015 to 30473d3 Compare January 1, 2020 01:53
@coolreader18
Copy link
Member

Thanks for contributing!

@coolreader18 coolreader18 merged commit 31d3fc6 into RustPython:master Jan 1, 2020
@doyshinda doyshinda deleted the impl_format_for_float branch January 1, 2020 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants