Add MixedMeasureUnit struct for handling CLDR mixed units #6971
Conversation
# Description This commit introduces the `MixedMeasureUnit` struct, which allows for the representation of mixed measurement units, such as "foot-and-inch" and "kilometer-and-meter". The implementation includes a method to create a `MixedMeasureUnit` from a string representation, ensuring that only valid combinations of single units are accepted. Additionally, tests have been added to verify the functionality and error handling for invalid mixed units.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
| /// which is a combination of one or more single units used together to express a measurement. | ||
| /// | ||
| /// # Examples | ||
| /// - `meter` - a special case of a mixed unit that contains only one single unit. |
There was a problem hiding this comment.
nit: don't list the special case as the first example
| /// - `foot-and-inch` | ||
| /// - `kilometer-and-meter` | ||
| /// | ||
| /// Note: Compound units such as `meter-per-second` or units with a constant denominator are not supported in mixed units. |
There was a problem hiding this comment.
"units with a constant denominator" such as?
There was a problem hiding this comment.
liter-per-100-kilometers falls under compound units, no? are there any units that have a purely constant denominator that this would apply to?
| pub fn try_from_str(mixed_units_str: &str) -> Result<MixedMeasureUnit, InvalidUnitError> { | ||
| // '-and-' is the separator for the mixed units and it is allowed to appear in the start or end of the string. | ||
| let mixed_units_strs = mixed_units_str.split("-and-"); | ||
| let mut mixed_units = SingleUnitVec::Zero; |
There was a problem hiding this comment.
nit: ::Zero should be ::Empty?
| use super::*; | ||
|
|
||
| #[test] | ||
| fn test_mixed_measure_unit_from_str() { |
There was a problem hiding this comment.
issue: what does kilometer-and-meter test that foot-and-inch doesn't? what does meter test that foot-and-inch doesn't?
the reason why I think excessive testing like this is an issue is that these tests don't test the whole construct-format cycle, which is the actual API. instead, they test internal representation, which might need to be changed in the future, and then you need to update every single one of these test cases.
I'd prefer if you added the whole construct-format functionality in the same PR and tested that instead of the internal representation.
|
|
3 participants
Description
This commit introduces:
MixedMeasureUnitstruct, which allows for the representation of mixed measurement units, such as "foot-and-inch" and "kilometer-and-meter".MixedMeasureUnitfrom a string representation, ensuring that only valid combinations of single units are accepted.