date time functions #425
date time functions #425
Conversation
BrianHung
commented
Jul 29, 2025
BrianHung
commented
- networkdays, networkdays.intl
- time, timevalue, hour, minute, second
- datedif, datevalue
- days, days360, weekday, weeknum, workday, workday.intl, yearfrac, isoweeknum
|
Hey, this one is amazing! I have deployed this branch to app.ironcalc.com and I'd like to merge this branch this week (I know, I know about freaking time) If you are still around one thing that would help is to have xlsx files like: https://github.com/ironcalc/IronCalc/blob/main/xlsx/tests/calc_tests/EOMONTH.xlsx But I might do them myself. Let's see how many of your PRs we mage to merge before the end of October! |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive set of date and time functions for IronCalc, significantly expanding its Excel compatibility. The changes include implementing 17 new functions (NETWORKDAYS, NETWORKDAYS.INTL, TIME, TIMEVALUE, HOUR, MINUTE, SECOND, DATEDIF, DATEVALUE, DAYS, DAYS360, WEEKDAY, WEEKNUM, WORKDAY, WORKDAY.INTL, YEARFRAC, ISOWEEKNUM) and updating their documentation to reflect their availability.
- Implemented 17 date and time functions with full Excel-compatible behavior including complex edge cases
- Updated function documentation from "not yet available" to "implemented but lacks detailed documentation" or added comprehensive documentation for key functions
- Added extensive test coverage for all new functions including boundary conditions, error handling, and Excel compatibility edge cases
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| base/src/functions/date_and_time.rs | Core implementation of all date/time functions with extensive helper functions for parsing and calculations |
| base/src/functions/mod.rs | Added function enum variants and wired them into the evaluation system |
| docs/src/functions/date-and-time.md | Updated status table to mark functions as available with documentation links |
| docs/src/functions/date_and_time/*.md | Updated individual function documentation pages to reflect implementation status |
| base/src/test/* | Added comprehensive test suites for all new functions |
| base/src/expressions/parser/static_analysis.rs | Added function signatures for static analysis |
Comments suppressed due to low confidence (1)
base/src/functions/date_and_time.rs:1
- The hardcoded count '215' in the iterator return type should be automatically updated when new functions are added. Consider using a macro or constant to avoid manual maintenance of this number.
use chrono::DateTime;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
base/src/functions/date_and_time.rs
Outdated
| if (0..=23).contains(&hour) && (0..=59).contains(&minute) && second == 60 { | ||
| // Check if this normalizes to exactly 24:00:00 | ||
| let total_seconds = hour * 3600 + minute * 60 + second; | ||
| return total_seconds == 86400; // Exactly 24:00:00 |
There was a problem hiding this comment.
Magic number 86400 appears multiple times throughout the code. Consider defining a constant like const SECONDS_PER_DAY: i32 = 86400 to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
| &self, | ||
| serial: i64, | ||
| cell: CellReferenceIndex, | ||
| ) -> Result<chrono::NaiveDate, CalcResult> { | ||
| match from_excel_date(serial) { | ||
| Ok(date) => Ok(date), | ||
| Err(_) => Err(CalcResult::Error { | ||
| error: Error::NUM, | ||
| origin: cell, | ||
| message: "Out of range parameters for date".to_string(), | ||
| }), | ||
| } | ||
| } |
There was a problem hiding this comment.
The error message 'Out of range parameters for date' is duplicated in multiple places. Consider defining a constant for this error message to ensure consistency and easier maintenance.
Copilot uses AI. Check for mistakes.
| const MIDNIGHT: &str = "0"; // 00:00:00 = 0/24 = 0 | ||
| const NOON: &str = "0.5"; // 12:00:00 = 12/24 = 0.5 | ||
| const TIME_14_30: &str = "0.604166667"; // 14:30:00 = 14.5/24 ≈ 0.604166667 | ||
| const TIME_14_30_45: &str = "0.6046875"; // 14:30:45 = 14.5125/24 = 0.6046875 |
There was a problem hiding this comment.
[nitpick] The comments show calculations but some precision may be lost in the string representation. Consider adding a note about precision limitations or use more precise decimal representations if needed for test accuracy.
| const MIDNIGHT: &str = "0"; // 00:00:00 = 0/24 = 0 | |
| const NOON: &str = "0.5"; // 12:00:00 = 12/24 = 0.5 | |
| const TIME_14_30: &str = "0.604166667"; // 14:30:00 = 14.5/24 ≈ 0.604166667 | |
| const TIME_14_30_45: &str = "0.6046875"; // 14:30:45 = 14.5125/24 = 0.6046875 | |
| // NOTE: The string representations of time values below are rounded/truncated for readability and test purposes. | |
| // Some precision may be lost compared to the full floating-point calculation. | |
| const MIDNIGHT: &str = "0"; // 00:00:00 = 0/24 = 0 | |
| const NOON: &str = "0.5"; // 12:00:00 = 12/24 = 0.5 | |
| const TIME_14_30: &str = "0.604166667"; // 14:30:00 = 14.5/24 ≈ 0.604166667 | |
| const TIME_14_30_45: &str = "0.6046875"; // 14:30:45 = 14.5125/24 ≈ 0.6046875 |
Copilot uses AI. Check for mistakes.
