Skip to content
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

Removes vestigial dependency on BigDecimal #594

Merged
merged 3 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ experimental-lazy-reader = []

[dependencies]
base64 = "0.12"
bigdecimal = "0.3.0"
bytes = "0.4"
# chrono < 0.5 brings in a deprecated version of the `time` crate via `oldtime` feature by default
# this makes it explicitly not do this as there is an advisory warning against this:
Expand Down
2 changes: 1 addition & 1 deletion src/binary/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::io::Write;

use arrayvec::ArrayVec;
use bigdecimal::Zero;
use num_traits::Zero;

use crate::binary::int::DecodedInt;
use crate::binary::raw_binary_writer::MAX_INLINE_LENGTH;
Expand Down
96 changes: 60 additions & 36 deletions src/element/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,16 +216,13 @@ impl<'a, R: IonReader<Item = StreamItem, Symbol = Symbol> + ?Sized> ElementLoade
#[cfg(test)]
mod reader_tests {
use super::*;
use crate::element::Value::*;
use crate::element::{Element, IntoAnnotatedElement};
use crate::ion_data::IonEq;
use crate::types::{Int, Timestamp as TS};
use crate::types::{Decimal, Timestamp};
use crate::{ion_list, ion_seq, ion_sexp, ion_struct};
use crate::{IonType, Symbol};
use bigdecimal::BigDecimal;
use num_bigint::BigInt;
use rstest::*;
use std::str::FromStr;

#[rstest]
#[case::nulls(
Expand All @@ -245,20 +242,22 @@ mod reader_tests {
null.struct
"#,
vec![
Null(IonType::Null),
Null(IonType::Bool),
Null(IonType::Int),
Null(IonType::Float),
Null(IonType::Decimal),
Null(IonType::Timestamp),
Null(IonType::Symbol),
Null(IonType::String),
Null(IonType::Clob),
Null(IonType::Blob),
Null(IonType::List),
Null(IonType::SExp),
Null(IonType::Struct),
].into_iter().map(|v| v.into()).collect(),
IonType::Null,
IonType::Bool,
IonType::Int,
IonType::Float,
IonType::Decimal,
IonType::Timestamp,
IonType::Symbol,
IonType::String,
IonType::Clob,
IonType::Blob,
IonType::List,
IonType::SExp,
IonType::Struct,
].into_iter()
.map(Element::from)
.collect(),
)]
#[case::ints(
br#"
Expand All @@ -274,43 +273,57 @@ mod reader_tests {
-65536, 65535,
-4294967296, 4294967295,
-9007199254740992, 9007199254740991,
].into_iter().map(Int::from).chain(
vec![
].into_iter()
.map(Element::from)
.chain(
vec![
"-18446744073709551616", "18446744073709551615",
"-79228162514264337593543950336", "79228162514264337593543950335",
].into_iter()
.map(|v| Int::from(BigInt::parse_bytes(v.as_bytes(), 10).unwrap()))
).map(|ai| Int(ai).into()).collect(),
.map(|v| BigInt::parse_bytes(v.as_bytes(), 10).unwrap())
.map(Element::from)
)
.collect(),
)]
#[case::int64_threshold_as_big_int(
&[0xE0, 0x01, 0x00, 0xEA, 0x28, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF],
vec![
"18446744073709551615",
].into_iter()
.map(|v| Int::from(BigInt::parse_bytes(v.as_bytes(), 10).unwrap())).map(|ai| Int(ai).into()).collect(),
.map(|v| BigInt::parse_bytes(v.as_bytes(), 10).unwrap())
.map(Element::from)
.collect(),
)]
#[case::int64_threshold_as_int64(
&[0xE0, 0x01, 0x00, 0xEA, 0x38, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00],
vec![
"-9223372036854775808",
].into_iter()
.map(|v| Int::from(BigInt::parse_bytes(v.as_bytes(), 10).unwrap())).map(|ai| Int(ai).into()).collect(),
.map(|v| BigInt::parse_bytes(v.as_bytes(), 10).unwrap())
.map(Element::from)
.collect(),
)]
#[case::floats(
br#"
1e0 +inf -inf nan
"#,
vec![
1f64, f64::INFINITY, f64::NEG_INFINITY, f64::NAN
].into_iter().map(|v| Float(v).into()).collect(),
].into_iter()
.map(Element::from)
.collect(),
)]
#[case::decimals(
br#"
1d0 100d10 -2.1234567d-100
"#,
vec![
"1e0", "100e10", "-2.1234567e-100",
].into_iter().map(|s| Decimal(BigDecimal::from_str(s).unwrap().into()).into()).collect(),
Decimal::new(1, 0),
Decimal::new(100, 10),
Decimal::new(-21234567, -107),
].into_iter()
.map(Element::from)
.collect(),
)]
#[case::timestamps(
br#"
Expand All @@ -320,16 +333,19 @@ mod reader_tests {
2020-02-27T14:16:33.123Z
"#,
vec![
TS::with_year(2020).build(),
TS::with_ymd(2020, 2, 27).build(),
TS::with_ymd(2020, 2, 27)
Timestamp::with_year(2020).build(),
Timestamp::with_ymd(2020, 2, 27).build(),
Timestamp::with_ymd(2020, 2, 27)
.with_hms(14, 16, 33)
.build_at_unknown_offset(),
TS::with_ymd(2020, 2, 27)
Timestamp::with_ymd(2020, 2, 27)
.with_hms(14, 16, 33)
.with_milliseconds(123)
.build_at_offset(0),
].into_iter().map(|ts_res| Timestamp(ts_res.unwrap()).into()).collect(),
].into_iter()
.map(Result::unwrap)
.map(Element::from)
.collect(),
)]
#[case::text_symbols(
br#"
Expand All @@ -338,7 +354,9 @@ mod reader_tests {
"#,
vec![
"foo", "bar",
].into_iter().map(|s| Symbol(s.into()).into()).collect(),
].into_iter()
.map(Element::symbol)
.collect(),
)]
#[case::strings(
br#"
Expand All @@ -347,7 +365,9 @@ mod reader_tests {
"#,
vec![
"hello", "world",
].into_iter().map(|s| String(s.into()).into()).collect(),
].into_iter()
.map(Element::from)
.collect(),
)]
#[case::clobs(
br#"
Expand All @@ -360,7 +380,9 @@ mod reader_tests {
b"goodbye", b"moon",
];
lobs
}.into_iter().map(|b| Clob(b.into()).into()).collect(),
}.into_iter()
.map(Element::clob)
.collect(),
)]
#[case::blobs(
br#"
Expand All @@ -372,7 +394,9 @@ mod reader_tests {
b"moo",
];
lobs
}.into_iter().map(|b| Blob(b.into()).into()).collect(),
}.into_iter()
.map(Element::blob)
.collect(),
)]
#[case::lists(
br#"
Expand Down
7 changes: 0 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,6 @@ pub use {

pub use result::{IonError, IonResult};

/// Re-exports of third party dependencies that are part of our public API.
///
/// See also: <https://github.com/amazon-ion/ion-rust/issues/302>
pub mod external {
pub use bigdecimal;
}

/// Whether or not the text spacing is generous/human-friendly or something more compact.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[non_exhaustive]
Expand Down
20 changes: 0 additions & 20 deletions src/text/raw_text_writer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::io::{BufWriter, Write};

use bigdecimal::BigDecimal;
use chrono::{DateTime, FixedOffset};

use crate::ion_writer::IonWriter;
Expand Down Expand Up @@ -380,14 +379,6 @@ impl<W: Write> RawTextWriter<W> {
Ok(())
}

/// Writes the provided BigDecimal value as an Ion decimal.
pub fn write_big_decimal(&mut self, value: &BigDecimal) -> IonResult<()> {
self.write_scalar(|output| {
write!(output, "{}", &value)?;
Ok(())
})
}

/// Writes the provided DateTime value as an Ion timestamp.
#[deprecated(
since = "0.6.1",
Expand Down Expand Up @@ -746,9 +737,7 @@ impl<W: Write> IonWriter for RawTextWriter<W> {
#[cfg(test)]
mod tests {
use std::str;
use std::str::FromStr;

use bigdecimal::BigDecimal;
use chrono::{FixedOffset, NaiveDate, TimeZone};

use crate::ion_writer::IonWriter;
Expand Down Expand Up @@ -858,15 +847,6 @@ mod tests {
);
}

#[test]
fn write_decimal() {
let decimal_text = "731221.9948";
write_scalar_test(
|w| w.write_big_decimal(&BigDecimal::from_str(decimal_text).unwrap()),
decimal_text,
);
}

#[test]
fn write_datetime_epoch() {
#![allow(deprecated)] // `write_datetime` is deprecated
Expand Down
64 changes: 1 addition & 63 deletions src/types/decimal/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::cmp::{max, Ordering};

use bigdecimal::{BigDecimal, Signed};
use num_bigint::{BigInt, BigUint, ToBigInt, ToBigUint};

use crate::ion_data::{IonEq, IonOrd};
Expand Down Expand Up @@ -494,44 +493,13 @@ impl Display for Decimal {
}
}

/// Make a Decimal from a BigDecimal. This is a lossless operation.
impl From<BigDecimal> for Decimal {
fn from(value: BigDecimal) -> Self {
let sign = if value.sign() == num_bigint::Sign::Minus {
Sign::Negative
} else {
Sign::Positive
};
let (big_int_coefficient, negative_exponent) = value.as_bigint_and_exponent();
// Discard the BigInt coefficient's sign before converting it to a BigUint to ensure
// the conversion succeeds.
let magnitude: BigUint = big_int_coefficient.abs().to_biguint().unwrap();
// From the BigInt docs: "Note that a positive exponent indicates a negative power of 10."
let exponent = -negative_exponent;

Decimal::new(Coefficient::new(sign, magnitude), exponent)
}
}

impl TryFrom<Decimal> for BigDecimal {
type Error = IonError;
/// Attempts to create a BigDecimal from a Decimal. Returns an Error if the Decimal being
/// converted is a negative zero, which BigDecimal cannot represent. Returns Ok otherwise.
fn try_from(value: Decimal) -> Result<Self, Self::Error> {
// The Coefficient type cannot be converted to a BigInt if it is a negative zero.
let coefficient_big_int: BigInt = value.coefficient.try_into()?;
Ok(BigDecimal::new(coefficient_big_int, -value.exponent))
}
}

#[cfg(test)]
mod decimal_tests {
use crate::result::IonResult;
use crate::types::{Coefficient, Decimal, Int, Sign, UInt};
use bigdecimal::BigDecimal;
use num_bigint::{BigInt, BigUint};

use num_traits::{Float, ToPrimitive};
use num_traits::Float;
use std::cmp::Ordering;
use std::convert::TryInto;
use std::fmt::Write;
Expand Down Expand Up @@ -780,36 +748,6 @@ mod decimal_tests {
assert!(conversion_result.is_err());
}

#[test]
fn test_convert_to_big_decimal() {
let decimal = Decimal::new(-24601, -3);
let big_decimal: BigDecimal = decimal.try_into().unwrap();
let double = big_decimal.to_f64().unwrap();
assert_eq!(-24.601, double);

// Any form of negative zero will fail to be converted.

let decimal = Decimal::negative_zero();
let conversion_result: IonResult<BigDecimal> = decimal.try_into();
assert!(conversion_result.is_err());

let decimal = Decimal::negative_zero_with_exponent(6);
let conversion_result: IonResult<BigDecimal> = decimal.try_into();
assert!(conversion_result.is_err());

let decimal = Decimal::negative_zero_with_exponent(-6);
let conversion_result: IonResult<BigDecimal> = decimal.try_into();
assert!(conversion_result.is_err());
}

#[test]
fn test_convert_from_big_decimal() {
let big_decimal: BigDecimal = BigDecimal::new((-24601).into(), 3);
let actual: Decimal = big_decimal.into();
let expected = Decimal::new(-24601, -3);
assert_eq!(actual, expected);
}

#[rstest]
#[case(Decimal::new(-24601, -3), 3)]
#[case(Decimal::new(u64::MAX, -5), 5)]
Expand Down
17 changes: 0 additions & 17 deletions tests/reexport_external.rs

This file was deleted.

Loading