Add support for the % format code for floats.#1657
Add support for the % format code for floats.#1657coolreader18 merged 1 commit intoRustPython:masterfrom
% format code for floats.#1657Conversation
2007961 to
410f82f
Compare
coolreader18
left a comment
There was a problem hiding this comment.
Looks good! I made a few comments and once those are addressed this is ready to merge.
tests/snippets/strings.py
Outdated
| assert f'{-10.0:.8%}' == '-1000.00000000%' | ||
| assert '{:%}'.format(float('nan')) == 'nan%' | ||
| assert '{:%}'.format(float('NaN')) == 'nan%' | ||
| assert '{:%}'.format(float('NAN')) == 'nan%' |
There was a problem hiding this comment.
The difference in casing is kinda redundant, as the formatting machinery isn't really involved; it's float.__new__ that parses the nans.
There was a problem hiding this comment.
Indeed it is, will remove 👍
vm/src/obj/objfloat.rs
Outdated
| let format_spec = FormatSpec::parse(spec.as_str())?; | ||
| format_spec.format_float(self.value) | ||
| }; | ||
| match try_format() { |
There was a problem hiding this comment.
You could do something like
match FormatSpec::parse(spec.as_str()).and_then(|spec| spec.format_float(self.value)) {
...
}to avoid the closure.
There was a problem hiding this comment.
Sure thing. Out of curiosity, what are the benefits (if any) of doing as shown above vs. using a closure? Or is it just Rust convention?
There was a problem hiding this comment.
Since it's not a very complicated operation, it's simpler to use the combinator method than define a closure and immediately call it once. It's similar to do vs >>= if you've worked with Haskell (do notation considered harmful).
| let (format_type, after_format_type) = parse_format_type(after_precision); | ||
| if !after_format_type.is_empty() { | ||
| return Err("Invalid format spec"); | ||
| } |
There was a problem hiding this comment.
Maybe add some tests that ensure that this properly fails with characters left after the format type.
Contributes to RustPython#1656
410f82f to
765089d
Compare
|
Thanks for contributing! |
Contributes to #1656