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.

Sign up for GitHub

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

Jump to bottom

Add more functions to CompactDecimalFormatter #3699

Merged
merged 8 commits into from
Sep 21, 2023
Merged

Add more functions to CompactDecimalFormatter #3699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
60e900d
Add format_fixed_decimal to CompactDecimalFormatter
sffc Jul 18, 2023
2df9c58
Add format_f64 and other helpers
sffc Jul 18, 2023
90a8970
Docs fix
sffc Aug 26, 2023
a633485
Fix up rust links
sffc Aug 26, 2023
3789d14
diplomat-gen
sffc Aug 26, 2023
b431bf5
Merge branch 'unicode-org:main' into fmt_compact_fd
sffc Aug 29, 2023
b352c49
Merge branch 'main' into fmt_compact_fd
sffc Sep 20, 2023
b050ed4
Document required feature
sffc Sep 20, 2023
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: 1 addition & 0 deletions experimental/compactdecimal/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,4 @@ std = ["fixed_decimal/std", "icu_decimal/std", "icu_plurals/std", "icu_provider/
serde = ["dep:serde", "zerovec/serde", "icu_decimal/serde", "icu_plurals/serde"]
datagen = ["std", "serde", "dep:databake", "zerovec/databake"]
compiled_data = ["dep:icu_compactdecimal_data", "dep:icu_locid_transform", "icu_decimal/compiled_data", "icu_plurals/compiled_data"]
ryu = ["fixed_decimal/ryu"]
174 changes: 164 additions & 10 deletions experimental/compactdecimal/src/compactdecimal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,15 +278,19 @@ impl CompactDecimalFormatter {
/// The result may have a fractional digit only if it is compact and its
/// significand is less than 10. Trailing fractional 0s are omitted, and
/// a sign is shown only for negative values.
///
/// # Examples
///
/// ```
/// # use icu_compactdecimal::CompactDecimalFormatter;
/// # use icu_locid::locale;
/// # use writeable::assert_writeable_eq;
/// #
/// # let short_english = CompactDecimalFormatter::try_new_short(
/// # &locale!("en").into(),
/// # Default::default(),
/// # ).unwrap();
/// use icu_compactdecimal::CompactDecimalFormatter;
/// use icu_locid::locale;
/// use writeable::assert_writeable_eq;
///
/// let short_english = CompactDecimalFormatter::try_new_short(
/// &locale!("en").into(),
/// Default::default(),
/// ).unwrap();
///
/// assert_writeable_eq!(short_english.format_i64(0), "0");
/// assert_writeable_eq!(short_english.format_i64(2), "2");
/// assert_writeable_eq!(short_english.format_i64(843), "843");
Expand All @@ -295,8 +299,10 @@ impl CompactDecimalFormatter {
/// assert_writeable_eq!(short_english.format_i64(3_010_349), "3M");
/// assert_writeable_eq!(short_english.format_i64(-13_132), "-13K");
/// ```
///
/// The result is the nearest such compact number, with halfway cases-
/// rounded towards the number with an even least significant digit.
///
/// ```
/// # use icu_compactdecimal::CompactDecimalFormatter;
/// # use icu_locid::locale;
Expand All @@ -315,9 +321,156 @@ impl CompactDecimalFormatter {
/// ```
pub fn format_i64(&self, value: i64) -> FormattedCompactDecimal<'_> {
let unrounded = FixedDecimal::from(value);
let log10_type = unrounded.nonzero_magnitude_start();
self.format_fixed_decimal(unrounded)
}

/// Formats a floating-point number in compact decimal notation using the default
/// precision settings.
Copy link
Member

Choose a reason for hiding this comment

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

Do I understand correctly that if you want something fancier, e.g., at least three digits (like YT subscriber counts), or directed rounding (like everything at YT) you then need to round yourself?

(That works for me.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Would you be okay if we landed this PR and then in the future we added such options to CompactDecimalFormatterOptions?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, though the rounding direction discussion would then come back to haunt us :-)

As I wrote, I am also fine with just not having those options, and seeing the sort of thing that YouTube does as an advanced do-your-own-rounding scenario; since we have format_compact_decimal and CompactDecimal, it is much easier for the caller to have their own arbitrarily fancy rounding logic than it was for me with ICU a few years ago.

Copy link
Member Author

Choose a reason for hiding this comment

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

Follow-up filed: #3929

///
/// The result may have a fractional digit only if it is compact and its
/// significand is less than 10. Trailing fractional 0s are omitted, and
/// a sign is shown only for negative values.
///
Copy link
Contributor

Choose a reason for hiding this comment

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

This should document that the ryu feature is needed

/// ✨ *Enabled with the `ryu` Cargo feature.*
///
/// # Examples
///
/// ```
/// use icu_compactdecimal::CompactDecimalFormatter;
/// use icu_locid::locale;
/// use writeable::assert_writeable_eq;
///
/// let short_english = CompactDecimalFormatter::try_new_short(
/// &locale!("en").into(),
/// Default::default(),
/// ).unwrap();
///
/// assert_writeable_eq!(short_english.format_f64(0.0).unwrap(), "0");
/// assert_writeable_eq!(short_english.format_f64(2.0).unwrap(), "2");
/// assert_writeable_eq!(short_english.format_f64(843.0).unwrap(), "843");
/// assert_writeable_eq!(short_english.format_f64(2207.0).unwrap(), "2.2K");
/// assert_writeable_eq!(short_english.format_f64(15_127.0).unwrap(), "15K");
/// assert_writeable_eq!(short_english.format_f64(3_010_349.0).unwrap(), "3M");
/// assert_writeable_eq!(short_english.format_f64(-13_132.0).unwrap(), "-13K");
/// ```
///
/// The result is the nearest such compact number, with halfway cases-
/// rounded towards the number with an even least significant digit.
///
/// ```
/// # use icu_compactdecimal::CompactDecimalFormatter;
/// # use icu_locid::locale;
/// # use writeable::assert_writeable_eq;
/// #
/// # let short_english = CompactDecimalFormatter::try_new_short(
/// # &locale!("en").into(),
/// # Default::default(),
/// # ).unwrap();
/// assert_writeable_eq!(short_english.format_f64(999_499.99).unwrap(), "999K");
/// assert_writeable_eq!(short_english.format_f64(999_500.00).unwrap(), "1M");
/// assert_writeable_eq!(short_english.format_f64(1650.0).unwrap(), "1.6K");
/// assert_writeable_eq!(short_english.format_f64(1750.0).unwrap(), "1.8K");
/// assert_writeable_eq!(short_english.format_f64(1950.0).unwrap(), "2K");
/// assert_writeable_eq!(short_english.format_f64(-1_172_700.0).unwrap(), "-1.2M");
/// ```
#[cfg(feature = "ryu")]
pub fn format_f64(
&self,
value: f64,
) -> Result<FormattedCompactDecimal<'_>, CompactDecimalError> {
use fixed_decimal::FloatPrecision::Floating;
// NOTE: This first gets the shortest representation of the f64, which
// manifests as double rounding.
Copy link
Member

@eggrobin eggrobin Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don’t think this is actually a problem given that you do not give the user control over the rounding mode, and that the second rounding is only in the compact case: the two roundings are nearest ties to even, and the ties for the second rounding are representable unless you have really stupidly large numbers (being integers), so the first rounding will not produce a tie for the second.

The double rounding issue would manifest if you could request things such as « nearest, ties to even, exactly one sig. dec. »: then 95.0 / 100.0, which is a little below 0.95, would round to 0.95 in the first rounding, and to 1.0 in the second, whereas it ought to round to 0.9.

let partly_rounded = FixedDecimal::try_from_f64(value, Floating)?;
Ok(self.format_fixed_decimal(partly_rounded))
}

/// Formats a [`FixedDecimal`] by automatically scaling and rounding it.
///
/// The result may have a fractional digit only if it is compact and its
/// significand is less than 10. Trailing fractional 0s are omitted.
///
/// Because the FixedDecimal is mutated before formatting, this function
/// takes ownership of it.
///
/// # Examples
///
/// ```
/// use fixed_decimal::FixedDecimal;
/// use icu_compactdecimal::CompactDecimalFormatter;
/// use icu_locid::locale;
/// use writeable::assert_writeable_eq;
///
/// let short_english = CompactDecimalFormatter::try_new_short(
/// &locale!("en").into(),
/// Default::default(),
/// ).unwrap();
///
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(0)),
/// "0");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(2)),
/// "2");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(843)),
/// "843");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(2207)),
/// "2.2K");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(15127)),
/// "15K");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(3010349)),
/// "3M");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(-13132)),
/// "-13K");
///
/// // The sign display on the FixedDecimal is respected:
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal(FixedDecimal::from(2500)
/// .with_sign_display(fixed_decimal::SignDisplay::ExceptZero)),
/// "+2.5K");
/// ```
///
/// The result is the nearest such compact number, with halfway cases-
/// rounded towards the number with an even least significant digit.
///
/// ```
/// # use fixed_decimal::FixedDecimal;
/// # use icu_compactdecimal::CompactDecimalFormatter;
/// # use icu_locid::locale;
/// # use writeable::assert_writeable_eq;
/// #
/// # let short_english = CompactDecimalFormatter::try_new_short(
/// # &locale!("en").into(),
/// # Default::default(),
/// # ).unwrap();
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal("999499.99".parse().unwrap()),
/// "999K");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal("999500.00".parse().unwrap()),
/// "1M");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal("1650".parse().unwrap()),
/// "1.6K");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal("1750".parse().unwrap()),
/// "1.8K");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal("1950".parse().unwrap()),
/// "2K");
/// assert_writeable_eq!(
/// short_english.format_fixed_decimal("-1172700".parse().unwrap()),
/// "-1.2M");
/// ```
pub fn format_fixed_decimal(&self, value: FixedDecimal) -> FormattedCompactDecimal<'_> {
let log10_type = value.nonzero_magnitude_start();
let (mut plural_map, mut exponent) = self.plural_map_and_exponent_for_magnitude(log10_type);
let mut significand = unrounded.multiplied_pow10(-i16::from(exponent));
let mut significand = value.multiplied_pow10(-i16::from(exponent));
// If we have just one digit before the decimal point…
if significand.nonzero_magnitude_start() == 0 {
// …round to one fractional digit…
Expand Down Expand Up @@ -373,6 +526,7 @@ impl CompactDecimalFormatter {
/// be equal to `formatter.compact_exponent_for_magnitude(n.significand().nonzero_magnitude_start() + n.exponent())`.
///
/// # Examples
///
/// ```
/// # use icu_compactdecimal::CompactDecimalFormatter;
/// # use icu_locid::locale;
Expand Down
10 changes: 10 additions & 0 deletions experimental/compactdecimal/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

use displaydoc::Display;
use fixed_decimal::FixedDecimalError;
use icu_decimal::DecimalError;
use icu_plurals::PluralsError;
use icu_provider::DataError;
Expand All @@ -22,6 +23,9 @@ pub enum CompactDecimalError {
/// An error originating from [`FixedDecimalFormatter`](icu_decimal::FixedDecimalFormatter).
#[displaydoc("Error loading FixedDecimalFormatter: {0}")]
Decimal(DecimalError),
/// An error originating from [`FixedDecimal`](fixed_decimal::FixedDecimal).
#[displaydoc("Error creating FixedDecimal: {0}")]
FixedDecimal(FixedDecimalError),
/// An error due to a [`CompactDecimal`](fixed_decimal::CompactDecimal) with an
/// exponent inconsistent with the compact decimal data for the locale, e.g.,
/// when formatting 1c5 in English (US).
Expand Down Expand Up @@ -53,3 +57,9 @@ impl From<DecimalError> for CompactDecimalError {
CompactDecimalError::Decimal(e)
}
}

impl From<FixedDecimalError> for CompactDecimalError {
fn from(e: FixedDecimalError) -> Self {
CompactDecimalError::FixedDecimal(e)
}
}
26 changes: 26 additions & 0 deletions experimental/compactdecimal/src/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,32 @@ pub struct FormattedCompactDecimal<'l> {
pub(crate) plural_map: Option<ZeroMap2dCursor<'l, 'l, i8, Count, PatternULE>>,
}

impl FormattedCompactDecimal<'_> {
/// Access the resolved [`CompactDecimal`] after formatting.
///
/// # Examples
///
/// ```
/// use fixed_decimal::FixedDecimal;
/// use icu_compactdecimal::CompactDecimalFormatter;
/// use icu_locid::locale;
/// use writeable::assert_writeable_eq;
///
/// let short_english = CompactDecimalFormatter::try_new_short(
/// &locale!("en").into(),
/// Default::default(),
/// ).unwrap();
///
/// let formatted_compact_decimal = short_english.format_i64(2207);
///
/// assert_writeable_eq!(formatted_compact_decimal, "2.2K");
/// assert_eq!(formatted_compact_decimal.get_compact_decimal().to_string(), "2.2c3");
/// ```
pub fn get_compact_decimal(&self) -> &CompactDecimal {
&self.value
}
}

impl<'l> Writeable for FormattedCompactDecimal<'l> {
fn write_to<W>(&self, sink: &mut W) -> core::result::Result<(), core::fmt::Error>
where
Expand Down
Loading