dankelley / plan

R package for project planning
https://dankelley.github.io/plan/
33 stars 9 forks source link

"Ambiguous format" error when providing POSIXct to burndown chart's plot method as t.stop #23

Closed frankschmitt closed 5 years ago

frankschmitt commented 5 years ago

When providing a POSIXct argument to the plot method's t.stop parameter:


stop     = as.POSIXct(strptime("2020-01-31", "%Y-%m-%d"))
bd = ... # initialize burndown object
plot(bd, t.stop = stop)

the plot cannot be rendered due to the following error:

Warning: Error in charToDate: character string is not in a standard unambiguous format

Apparently, only char arguments are accepted, i.e. this works:

stop     = "2020-01-31"
bd = ... # initialize burndown object
plot(bd, t.stop = stop)

The relevant part of burndown.R:

if (t.stop != "") {
                  time.max = as.POSIXct(t.stop)
              } else {
                  time.max = x[["deadline"]]
              }

The documentation states:

t.stop | a POSIX time, the maximum time for graph (defaults to deadline if not given).

which seems to indicate that providing a POSIXct value should actually work. Is this perhaps time zone related? (printing stop in the first approach gives me ""2020-01-31 CET")

dankelley commented 5 years ago

I'll take a look.

dankelley commented 5 years ago

This should work now, in the "master" branch. Also, please note that I've made the test suite better, using testthat, and there are tests that ensure that the present bug is fixed, whether t.stop is character or POSIXt.

frankschmitt commented 5 years ago

Terrific! I really like the switch to testthat; it makes testing via devtools in RStudio a breeze. I'd like to add some tests for as.burndown() and put in another PR for that if it's ok with you.

dankelley commented 5 years ago

Yes, please -- the more tests, the better.