From d508f8ad7cea10663ec33337faaf125a0798022b Mon Sep 17 00:00:00 2001 From: MinGyo Jung Date: Thu, 15 Aug 2019 17:36:11 +0900 Subject: [PATCH 1/6] #1224 Add test code for floating point formatting Not supports #.#f format --- tests/snippets/strings.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/snippets/strings.py b/tests/snippets/strings.py index ed5bfb972..fee880550 100644 --- a/tests/snippets/strings.py +++ b/tests/snippets/strings.py @@ -212,6 +212,11 @@ assert "%(first)s %(second)s" % {'second': 'World!', 'first': "Hello,"} == "Hell assert "%(key())s" % {'key()': 'aaa'} assert "%s %a %r" % (f, f, f) == "str(Foo) repr(Foo) repr(Foo)" assert "repr() shows quotes: %r; str() doesn't: %s" % ("test1", "test2") == "repr() shows quotes: 'test1'; str() doesn't: test2" +assert "%f" % (1.2345) == "1.234500" +assert "%+f" % (1.2345) == "+1.234500" +assert "% f" % (1.2345) == " 1.234500" +assert "%f" % (-1.2345) == "-1.234500" +assert "%f" % (1.23456789012) == "1.234568" assert_raises(TypeError, lambda: "My name is %s and I'm %(age)d years old" % ("Foo", 25), msg="format requires a mapping") assert_raises(TypeError, lambda: "My name is %(name)s" % "Foo", msg="format requires a mapping") From 1114d27e281392dbccb7c7382137840558ca74a1 Mon Sep 17 00:00:00 2001 From: MinGyo Jung Date: Thu, 15 Aug 2019 17:39:03 +0900 Subject: [PATCH 2/6] #1224 Add formating for floating point and test Just supports "%f" types --- vm/src/cformat.rs | 70 ++++++++++++++++++++++++++++++++++++++++++++ vm/src/obj/objstr.rs | 13 ++++++++ 2 files changed, 83 insertions(+) diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index 2b17b992a..0d86609bf 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -209,6 +209,41 @@ impl CFormatSpec { self.fill_string(format!("{}{}", prefix, magnitude_string), ' ', None) } } + + pub fn format_float(&self, num: f64) -> String { + let magnitude = num.abs(); + + let sign_string = match num.is_sign_positive() { + false => "-", + true => { + if self.flags.contains(CConversionFlags::SIGN_CHAR) { + "+" + } else if self.flags.contains(CConversionFlags::BLANK_SIGN) { + " " + } else { + "" + } + } + }; + + // TODO: Support precision + let magnitude_string = format!("{:.6}", magnitude); + + if self.flags.contains(CConversionFlags::ZERO_PAD) { + let fill_char = if !self.flags.contains(CConversionFlags::LEFT_ADJUST) { + '0' + } else { + ' ' + }; + format!( + "{}{}", + sign_string, + self.fill_string(magnitude_string, fill_char, Some(sign_string.chars().count())) + ) + } else { + self.fill_string(format!("{}{}", sign_string, magnitude_string), ' ', None) + } + } } #[derive(Debug, PartialEq)] @@ -761,6 +796,41 @@ mod tests { .format_number(&BigInt::from(0x1337)), "0x1337 ".to_string() ); + assert_eq!( + "%f" + .parse::() + .unwrap() + .format_float(f64::from(1.2345)), + "1.234500".to_string() + ); + assert_eq!( + "%+f" + .parse::() + .unwrap() + .format_float(f64::from(1.2345)), + "+1.234500".to_string() + ); + assert_eq!( + "% f" + .parse::() + .unwrap() + .format_float(f64::from(1.2345)), + " 1.234500".to_string() + ); + assert_eq!( + "%f" + .parse::() + .unwrap() + .format_float(f64::from(-1.2345)), + "-1.234500".to_string() + ); + assert_eq!( + "%f" + .parse::() + .unwrap() + .format_float(f64::from(1.2345678901)), + "1.234568".to_string() + ); } #[test] diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index ff609eaad..95b4d0f05 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -29,6 +29,7 @@ use crate::vm::VirtualMachine; use super::objbytes::PyBytes; use super::objdict::PyDict; use super::objint::{self, PyInt}; +use super::objfloat; use super::objiter; use super::objnone::PyNone; use super::objsequence::PySliceableSequence; @@ -1220,6 +1221,18 @@ fn do_cformat_specifier( } Ok(format_spec.format_number(objint::get_value(&obj))) } + CFormatType::Float(_) => { + if !objtype::isinstance(&obj, &vm.ctx.float_type()) { + let required_type_string = "an floating point"; + return Err(vm.new_type_error(format!( + "%{} format: {} is required, not {}", + format_spec.format_char, + required_type_string, + obj.class() + ))); + } + Ok(format_spec.format_float(objfloat::get_value(&obj))) + } CFormatType::Character => { let char_string = { if objtype::isinstance(&obj, &vm.ctx.int_type()) { From e1ac7297708a1ec1895533494d5895513c8edbc9 Mon Sep 17 00:00:00 2001 From: MinGyo Jung Date: Thu, 15 Aug 2019 18:52:57 +0900 Subject: [PATCH 3/6] #1224 Add floating point formatting test for int --- tests/snippets/strings.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/snippets/strings.py b/tests/snippets/strings.py index fee880550..f4bb11cab 100644 --- a/tests/snippets/strings.py +++ b/tests/snippets/strings.py @@ -217,6 +217,8 @@ assert "%+f" % (1.2345) == "+1.234500" assert "% f" % (1.2345) == " 1.234500" assert "%f" % (-1.2345) == "-1.234500" assert "%f" % (1.23456789012) == "1.234568" +assert "%f" % (123) == "123.000000" +assert "%f" % (-123) == "-123.000000" assert_raises(TypeError, lambda: "My name is %s and I'm %(age)d years old" % ("Foo", 25), msg="format requires a mapping") assert_raises(TypeError, lambda: "My name is %(name)s" % "Foo", msg="format requires a mapping") From 26168bc1e5009f4eca263204042404bb875b0222 Mon Sep 17 00:00:00 2001 From: MinGyo Jung Date: Thu, 15 Aug 2019 18:53:46 +0900 Subject: [PATCH 4/6] #1224 Add integer supporting for float formatting --- vm/src/cformat.rs | 14 ++++++++++++++ vm/src/obj/objstr.rs | 13 ++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index 0d86609bf..e372384a8 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -831,6 +831,20 @@ mod tests { .format_float(f64::from(1.2345678901)), "1.234568".to_string() ); + assert_eq!( + "%f" + .parse::() + .unwrap() + .format_number(&BigInt::from(123)), + "123.000000".to_string() + ); + assert_eq!( + "%f" + .parse::() + .unwrap() + .format_number(&BigInt::from(-123)), + "-123.000000".to_string() + ) } #[test] diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 95b4d0f05..1e05fb862 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -1222,16 +1222,19 @@ fn do_cformat_specifier( Ok(format_spec.format_number(objint::get_value(&obj))) } CFormatType::Float(_) => { - if !objtype::isinstance(&obj, &vm.ctx.float_type()) { - let required_type_string = "an floating point"; - return Err(vm.new_type_error(format!( + if objtype::isinstance(&obj, &vm.ctx.float_type()) { + Ok(format_spec.format_float(objfloat::get_value(&obj))) + } else if objtype::isinstance(&obj, &vm.ctx.int_type()){ + Ok(format_spec.format_float(objint::get_value(&obj).to_f64().unwrap())) + } else { + let required_type_string = "an floating point or integer"; + Err(vm.new_type_error(format!( "%{} format: {} is required, not {}", format_spec.format_char, required_type_string, obj.class() - ))); + ))) } - Ok(format_spec.format_float(objfloat::get_value(&obj))) } CFormatType::Character => { let char_string = { From f4e19889a18d7a8e8159cf1509a00f2c0c4c839b Mon Sep 17 00:00:00 2001 From: MinGyo Jung Date: Fri, 16 Aug 2019 15:38:56 +0900 Subject: [PATCH 5/6] #1224 Fix not suitable tests and solve CI failure --- vm/src/cformat.rs | 52 ++++++++++++++++++-------------------------- vm/src/obj/objstr.rs | 4 ++-- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index e372384a8..cac22a3a5 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -213,17 +213,16 @@ impl CFormatSpec { pub fn format_float(&self, num: f64) -> String { let magnitude = num.abs(); - let sign_string = match num.is_sign_positive() { - false => "-", - true => { - if self.flags.contains(CConversionFlags::SIGN_CHAR) { - "+" - } else if self.flags.contains(CConversionFlags::BLANK_SIGN) { - " " - } else { - "" - } + let sign_string = if num.is_sign_positive() { + if self.flags.contains(CConversionFlags::SIGN_CHAR) { + "+" + } else if self.flags.contains(CConversionFlags::BLANK_SIGN) { + " " + } else { + "" } + } else { + "-" }; // TODO: Support precision @@ -238,7 +237,11 @@ impl CFormatSpec { format!( "{}{}", sign_string, - self.fill_string(magnitude_string, fill_char, Some(sign_string.chars().count())) + self.fill_string( + magnitude_string, + fill_char, + Some(sign_string.chars().count()) + ) ) } else { self.fill_string(format!("{}{}", sign_string, magnitude_string), ' ', None) @@ -796,9 +799,12 @@ mod tests { .format_number(&BigInt::from(0x1337)), "0x1337 ".to_string() ); + } + + #[test] + fn test_parse_and_format_float() { assert_eq!( - "%f" - .parse::() + "%f".parse::() .unwrap() .format_float(f64::from(1.2345)), "1.234500".to_string() @@ -818,33 +824,17 @@ mod tests { " 1.234500".to_string() ); assert_eq!( - "%f" - .parse::() + "%f".parse::() .unwrap() .format_float(f64::from(-1.2345)), "-1.234500".to_string() ); assert_eq!( - "%f" - .parse::() + "%f".parse::() .unwrap() .format_float(f64::from(1.2345678901)), "1.234568".to_string() ); - assert_eq!( - "%f" - .parse::() - .unwrap() - .format_number(&BigInt::from(123)), - "123.000000".to_string() - ); - assert_eq!( - "%f" - .parse::() - .unwrap() - .format_number(&BigInt::from(-123)), - "-123.000000".to_string() - ) } #[test] diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 1e05fb862..13e26ffd9 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -28,8 +28,8 @@ use crate::vm::VirtualMachine; use super::objbytes::PyBytes; use super::objdict::PyDict; -use super::objint::{self, PyInt}; use super::objfloat; +use super::objint::{self, PyInt}; use super::objiter; use super::objnone::PyNone; use super::objsequence::PySliceableSequence; @@ -1224,7 +1224,7 @@ fn do_cformat_specifier( CFormatType::Float(_) => { if objtype::isinstance(&obj, &vm.ctx.float_type()) { Ok(format_spec.format_float(objfloat::get_value(&obj))) - } else if objtype::isinstance(&obj, &vm.ctx.int_type()){ + } else if objtype::isinstance(&obj, &vm.ctx.int_type()) { Ok(format_spec.format_float(objint::get_value(&obj).to_f64().unwrap())) } else { let required_type_string = "an floating point or integer"; From 2c6c2b80847c9b1c210fe1eb6efd66f863f698df Mon Sep 17 00:00:00 2001 From: MinGyo Jung Date: Fri, 16 Aug 2019 16:41:53 +0900 Subject: [PATCH 6/6] Remove unreachable pattern --- vm/src/obj/objstr.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs index 8f69fa749..c20ea5bb1 100644 --- a/vm/src/obj/objstr.rs +++ b/vm/src/obj/objstr.rs @@ -1262,10 +1262,6 @@ fn do_cformat_specifier( format_spec.precision = Some(CFormatQuantity::Amount(1)); Ok(format_spec.format_string(char_string)) } - _ => Err(vm.new_not_implemented_error(format!( - "Not yet implemented for %{}", - format_spec.format_char - ))), } }