Skip to content

Add doctests for experimental unit converter#8056

Open
sffc wants to merge 4 commits into
unicode-org:mainfrom
sffc:doctests-units-converter
Open

Add doctests for experimental unit converter#8056
sffc wants to merge 4 commits into
unicode-org:mainfrom
sffc:doctests-units-converter

Conversation

@sffc

@sffc sffc commented Jun 10, 2026

Copy link
Copy Markdown
Member

This PR adds doctests showing the status quo for the unit converter in the icu_experimental crate.

🤖 This pull request was created by an AI agent working with @sffc.

Changelog

N/A (docs only)

Adds detailed doctests to ConverterFactory, UnitsConverter, and the units module covering proportional, offset, and reciprocal conversions, as well as edge cases like incompatible units.

Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Comment thread components/experimental/src/units/converter.rs Outdated
Comment thread components/experimental/src/units/converter.rs
Comment thread components/experimental/src/units/converter_factory.rs Outdated
Comment thread components/experimental/src/units/converter_factory.rs Outdated
Comment thread components/experimental/src/units/converter_factory.rs Outdated
Comment thread components/experimental/src/units/converter_factory.rs Outdated
Remove self-evident comments, move converter creation down, and remove redundant doctest.

Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@sffc

sffc commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

🤖 This comment is written by an AI agent working with @sffc

The feedback on the PR has been addressed:

  • Removed self-evident comments from the ConverterFactory struct doctest.
  • Moved converter creation down in both ConverterFactory and UnitsConverter doctests to be immediately above the assertions.
  • Removed the redundant doctest from ConverterFactory::converter and added a link to the struct-level docs.

All doctests have been verified to pass locally.

sffc and others added 2 commits June 9, 2026 19:48
Simplify ConverterFactory doctest, add BigRational example to UnitsConverter, and remove redundant UnitsConverter::convert doctest.

Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Demonstrates factory reuse by creating both meter-to-foot and foot-to-meter converters from the same factory instance.

Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@sffc sffc marked this pull request as ready for review June 10, 2026 03:20
@sffc sffc requested a review from younies as a code owner June 10, 2026 03:20
@sffc sffc changed the title docs: Add comprehensive doctests for unit converter Add doctests for experimental unit converter Jun 10, 2026
@sffc

sffc commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

//! let converter = factory.converter::<f64>(&celsius, &fahrenheit).unwrap();
//!
//! // 0°C is exactly 32°F.
//! assert!((converter.convert(&0.0) - 32.0).abs() < 1e-9);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

to calculate the precision, you should use something like:

Suggested change
//! assert!((converter.convert(&0.0) - 32.0).abs() < 1e-9);
//! assert!(((converter.convert(&0.0) - 32.0).abs())/32.0 < 1e-9);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for precision:

$$ \left| \frac{\text{actual} - \text{expected}}{\text{expected}} \right| < \delta $$

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Acknowledged, but I'm hoping to just remove the error bars entirely after we land #8058

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants