tustvold commented on code in PR #5128:
URL: https://github.com/apache/arrow-rs/pull/5128#discussion_r1405990181
##########
arrow-cast/src/cast.rs:
##########
@@ -2642,7 +2642,18 @@ where
}
let integers = parts[0].trim_start_matches('0');
- let decimals = if parts.len() == 2 { parts[1] } else { "" };
+
+ let (decimals, negative) = if parts.len() == 2 {
+ (parts[1], integers.starts_with('-'))
Review Comment:
Should this after the trim_start_matches above? I think this will parse
`000-1.343`
##########
arrow-cast/src/cast.rs:
##########
@@ -8304,6 +8331,8 @@ mod tests {
Some(""),
Some(" "),
None,
+ Some("-1.23499999"),
+ Some("-1.23599999"),
Review Comment:
Perhaps worth testing -0.001 or similar?
##########
arrow-cast/src/cast.rs:
##########
@@ -2642,7 +2642,18 @@ where
}
let integers = parts[0].trim_start_matches('0');
- let decimals = if parts.len() == 2 { parts[1] } else { "" };
+
+ let (decimals, negative) = if parts.len() == 2 {
+ (parts[1], integers.starts_with('-'))
+ } else {
+ ("", false)
+ };
+
+ let integers = if negative {
+ integers.trim_start_matches('-')
Review Comment:
Should this be trim_start_matches or just a single strip_prefix?
##########
arrow-cast/src/cast.rs:
##########
@@ -2677,7 +2688,17 @@ where
i256::ZERO
};
- format!("{}", integers.add_wrapping(adjusted))
+ let integers = if negative {
+ integers.neg_wrapping()
+ } else {
+ integers
+ };
+
+ if negative {
+ format!("{}", integers.sub_wrapping(adjusted))
Review Comment:
Not related to this PR, but I wonder if we need to format to a string and
then convert back
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
