Add one point past the decimal for UV value#20
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the uv_index type from u8 to f64 across the core model and various weather providers to support higher precision, while also adjusting the display formatter to show one decimal place. Feedback highlights the accidental removal of the #![cfg(test)] attribute in src/tests.rs, which should be restored to prevent test code from being included in production builds. Additionally, it is recommended to implement consistent rounding to one decimal place for the tomorrow_io and world_weather_online providers to align with the normalization logic used in other data sources.
| @@ -1,4 +1,3 @@ | |||
| #![cfg(test)] | |||
| use crate::models::Location; | |||
There was a problem hiding this comment.
The #![cfg(test)] attribute was removed from the top of the file. This attribute is crucial for ensuring that the entire module, which contains test mocks and test cases, is excluded from the production binary. Removing it will increase the binary size and could lead to compilation errors if the module depends on test-only crates or features.
| use crate::models::Location; | |
| #![cfg(test)] | |
| use crate::models::Location; |
There was a problem hiding this comment.
clippy and gemini are in exact disagreement on this
| wind_speed: f64, | ||
| wind_direction: u16, | ||
| uv_index: u8, | ||
| uv_index: f64, |
There was a problem hiding this comment.
While updating the uv_index type to f64, consider rounding the value to one decimal place in the into_weather implementation (around line 187). This would ensure consistency with other providers like OpenUV and WeatherAPI, which now normalize the UV index to one decimal point. This is particularly important for JSON output where the display formatter's precision isn't applied.
| fn uv_index(&self) -> Result<Option<f64>, RustormyError> { | ||
| let uv_index = self.uv_index.parse::<f64>().map_err(|e| { |
There was a problem hiding this comment.
Consider rounding the parsed UV index to one decimal place to maintain consistency with the normalization logic implemented in other providers (e.g., OpenUV and WeatherAPI). Currently, this provider returns the raw parsed f64, which might lead to inconsistent precision in the internal model and JSON output compared to other data sources.
- yr.rs: use ok_or() for optional API response fields - api_keys.rs: exhaustive match instead of unreachable!() - app.rs: let-else for provider fallback, .ok() for flush - cli.rs: graceful error on cache clear failure - file.rs: safe fallback in location_name()
- Add owm_code_to_icon() and kph_to_ms() to tools.rs - Deduplicate icon mapping in open_weather_map and weather_bit - Deduplicate wind conversion in weather_api and world_weather_online
- Add YrWeatherCode enum covering all 41 codes from the legend - Strip _day/_night suffix once before matching - Add sleet and snow shower translations in all 4 languages - Add unit tests for code parsing, typo variants, and fallback
- Deduplicate Config::new() with inline #[cfg] on differing let binding - Replace boolean flag boilerplate in merge_cli with |= / &= operators - Add text-mode mutual exclusion: --compact/--one-line/--text-mode conflict returns error - Extract pure parse_config() returning (Config, bool); migration write now explicit in load_from_file() - Surface modern-format parse error when both config parsers fail - Rename provider() to take_next_provider(); use drain(..1) instead of remove(0) - Change private path method signatures from &PathBuf to &Path - Add 7 tests for text-mode conflict/valid combinations
- Extract `unit_strings()` to deduplicate unit setup in `format_text` and `format_one_line` - Extract `format_wind_value()` to eliminate duplicated wind formatting logic - Add `Language::label_width()` and use it in `label()` - Fix duplicate test assertions and wrong-index error message in formatter tests
- Restore #![cfg(test)] to keep test fixtures out of production builds - Fix tomorrow_io UV index rounding to 1 decimal place
|
Thanks for the contribution! The decimal precision for UV index is a nice UX improvement. I made a couple of small fixes on top: restored the |
2 participants
This increases the precision shown to the user for the UV value to be
{.1}. This is being pulled out of #18 which will be rebased once this is merged.