impute_time() truncates sub-hour UTC offsets: tz = "+0530" is recorded as +05:00
- Dominant language
- R
- Stars
- 18
- Forks
- 1
- PR merge metrics
- No merged PRs in 30d
Description
`impute_time.partial_time()` assigns the imputed offset with integer division:
```r
# R/impute.R
tzhour_na <- is.na(vctrs::field(impute_pttm, "pttm_mat")[, "tzhour"])
vctrs::field(impute_pttm, "pttm_mat")[tzhour_na, "tzhour"] <- tz %/% 60
```
`interpret_tz()` returns minutes, so `%/% 60` discards any part of the offset that is not a whole hour. `tzhour` is stored as a double and holds fractional offsets perfectly well elsewhere in the package, so the loss is entirely in this line.
### Reproducible
```r
library(parttime)
options(parttime.assume_tz_offset = NA) # so the offset is genuinely unknown
x <- as.parttime("2001-06-15")
impute_time_mid(x, tz = "+0530") # tzhour 5 should be 5.5
impute_time_mid(x, tz = "+0545") # tzhour 5 should be 5.75
impute_time_mid(x, tz = "-0430") # tzhour -5 should be -4.5
impute_time_mid(x, tz = "+1245") # tzhour 12 should be 12.75
impute_time_mid(x, tz = "-0400") # tzhour -4 correct, whole hour
```
This affects explicit offset strings, not only timezone names, so it is independent of how the offset was obtained.
### Effect
`%/%` is floor division, so the recorded offset is always rounded *west*, by up to 59 minutes:
| offset | minutes | `%/% 60` | correct | error |
|---|---|---|---|---|
| `+0530` | 330 | 5 | 5.5 | −0:30 |
| `+0545` | 345 | 5 | 5.75 | −0:45 |
| `-0430` | −270 | −5 | −4.5 | −0:30 |
| `-0545` | −345 | −6 | −5.75 | −0:15 |
A local time carrying an offset that is too far west resolves to a UTC instant that is correspondingly *late*, so `10:30 +05:30` — which is `05:00Z` — is recorded as `10:30 +05:00` and resolves to `05:30Z`.
Every sub-hour zone is affected: India and Sri Lanka (+05:30), Nepal (+05:45), Iran (+03:30), Afghanistan (+04:30), Myanmar (+06:30), Eucla (+08:45), Adelaide and Darwin (+09:30), Lord Howe (+10:30), Chatham (+12:45), Newfoundland (−03:30), Marquesas (−09:30).
The package's own defaults are all whole hours — `impute_time_min()` uses `"-1200"`, `impute_time_max()` uses `"+1400"`, `impute_time_mid()` uses `"GMT"` — so the defaults are unaffected and this only appears when a caller supplies a sub-hour `tz`.
### Fix
```r
vctrs::field(impute_pttm, "pttm_mat")[tzhour_na, "tzhour"] <- tz / 60
```
`tzhour` already carries fractional values elsewhere — casting a `POSIXct` in `Asia/Kathmandu` yields `tzhour = 5.75`, and `format()` renders it as `+05:45` — so nothing downstream needs to change to accommodate it.
I could not find any test that pins the current truncated values, so the change should not require test updates beyond adding coverage for it.
### Not affected
For completeness, two nearby things that look similar but are correct:
- `to_gmt()` uses `%/% 1` on `tzhour` for the hour, but carries the remainder into minutes on the following line (`x[, "min"] + x[, "tzhour"] %% 1 * 60`), so the fraction survives.
- `format()` pairs `%/% 1` with `%% 1 * 60` the same way, and renders `+05:45` and `+05:30` correctly.
One genuinely odd spot, though it appears unreachable: `impute_partial_time_to_chr()` formats the offset minutes as `tzhour %/% 1 * 60`, which yields `300` rather than `30` for a `+05:30` offset (and `300` for a whole-hour `+05:00` too). Nothing in the package appears to call that function, so it may just be dead code worth removing.
Issue created with the help of Claude Opus 5.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in R/impute.R at impute_time.partial_time(), where the missing tzhour value is assigned from interpret_tz(). Reproduce the issue with the documented impute_time_mid() calls for +0530, +0545, and -0430, then add coverage showing fractional offsets are preserved while whole-hour offsets remain unchanged. Done means the recorded tzhour values match the supplied offsets and the relevant package tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- r
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100