Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify format.POSIXlt() to improve bugzilla issue 17350 #187

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

eddelbuettel
Copy link

Context: r-devel/r-dev-day#83
Original bugzilla issue: https://bugs.r-project.org/show_bug.cgi?id=17350

Basic idea is to simplify by removing the calculation of np which has been seen to leading to unwanted truncation as discussed in the bugzilla report. This tested out fine on my machine but my svn checkout is no longer pristine so trying here too.

There is also a bit of whitespace change as the indentation was off for under Emacs/ESS which is unusual for base R code.

there is also a bit of whitespace change as the indentation was off for under
Emacs/ESS which is unusual for base R code.
As discussed and co-developed with @hturner
@hturner
Copy link
Member

hturner commented Nov 20, 2024

I update the patch following Martin Maechler's review. With this patch:

x <- as.POSIXct("2009-08-03 12:01:59") + 0:3/10

# options(digits.secs) and digits are now consistent for the default format

options(digits.secs = 3)
format(x)
#> [1] "2009-08-03 12:01:59.000" "2009-08-03 12:01:59.099"
#> [3] "2009-08-03 12:01:59.200" "2009-08-03 12:01:59.299"

options(digits.secs = 0)
format(x, digits = 3)
#> [1] "2009-08-03 12:01:59.000" "2009-08-03 12:01:59.099"
#> [3] "2009-08-03 12:01:59.200" "2009-08-03 12:01:59.299"

# "%OS" format is now consistent with the default format

options(digits.secs = 3)
format(x, format = "%Y-%m-%d %H:%M:%OS")
#> [1] "2009-08-03 12:01:59.000" "2009-08-03 12:01:59.099"
#> [3] "2009-08-03 12:01:59.200" "2009-08-03 12:01:59.299"

options(digits.secs = 0)
format(x, format = "%Y-%m-%d %H:%M:%OS", digits = 3)
#> [1] "2009-08-03 12:01:59.000" "2009-08-03 12:01:59.099"
#> [3] "2009-08-03 12:01:59.200" "2009-08-03 12:01:59.299"

# order of precedence:  options(digits.secs) < digits < "%OSn"

options(digits.secs = 4)
format(x, format = "%Y-%m-%d %H:%M:%OS")
#> [1] "2009-08-03 12:01:59.0000" "2009-08-03 12:01:59.0999"
#> [3] "2009-08-03 12:01:59.2000" "2009-08-03 12:01:59.2999"

options(digits.secs = 4)
format(x, format = "%Y-%m-%d %H:%M:%OS", digits = 3)
#> [1] "2009-08-03 12:01:59.000" "2009-08-03 12:01:59.099"
#> [3] "2009-08-03 12:01:59.200" "2009-08-03 12:01:59.299"

options(digits.secs = 4)
format(x, format = "%Y-%m-%d %H:%M:%OS2", digits = 3)
#> [1] "2009-08-03 12:01:59.00" "2009-08-03 12:01:59.09" "2009-08-03 12:01:59.20"
#> [4] "2009-08-03 12:01:59.29"

# true trailing zeros are dropped...

y <- as.POSIXlt(c("2009-08-03 12:01:59.000", "2009-08-03 12:01:59.090"))
format(y)
#> [1] "2009-08-03 12:01:59.00" "2009-08-03 12:01:59.09"

# ...even if we ask for more digits...

format(y, digits = 3)
#> [1] "2009-08-03 12:01:59.00" "2009-08-03 12:01:59.09"

# ... unless we hard code in the format

format(y, format = "%Y-%m-%d %H:%M:%OS3", digits = 3)
#> [1] "2009-08-03 12:01:59.000" "2009-08-03 12:01:59.090"

@hturner
Copy link
Member

hturner commented Nov 20, 2024

Also, it still drops to simpler format if possible

z <- as.POSIXlt(c("2009-08-03 12:01:59.000", "2009-08-03 12:01:59.000"))

options(digits.secs = 4)
format(z, format = "%Y-%m-%d %H:%M:%OS")
#> [1] "2009-08-03 12:01:59" "2009-08-03 12:01:59"

a <- as.POSIXlt(c("2009-08-03", "2009-08-03"))

options(digits.secs = 4)
format(a, format = "%Y-%m-%d %H:%M:%OS")
#> [1] "2009-08-03" "2009-08-03"

@eddelbuettel
Copy link
Author

eddelbuettel commented Nov 20, 2024

Nice work!

# true trailing zeros are dropped...

I am actually not sure I like this. Assume a situation where you have 'random' time measurements in a mix of 'full' and 'fractional' seconds. You will now get a display of data column with varying formats if a given pick just happens to have zeros only so the format is inconsistent. But I will shut up here and not veto as some people are clearly adamant about this -- and I guess there is an out in using %OSn with a given n as we honor that.

So maybe we need to language to strptime.Rd stressing that possibility? Current main branch has

Specific to \R is \code{%OSn}, which for output gives the seconds
truncated to \code{0 <= n <= 6} decimal places (and if \code{%OS} is
not followed by a digit, it uses \code{digits} unless that is
\code{NULL}, when \code{n = 0}). Further, for \code{strptime}
\code{%OS} will input seconds including fractional seconds. Note that
\code{%S} does not read fractional parts on output.

which still mentions truncation but we don't now -- as you show at the end

# ... unless we hard code in the format

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