hubverse-org / hubUtils

Utility functions for Infectious Disease Modeling Hubs
https://hubverse-org.github.io/hubUtils/
Other
6 stars 3 forks source link

`create_task_id()` for `origin_date` returns an error if required = Date object #121

Closed LucieContamin closed 8 months ago

LucieContamin commented 11 months ago

I am currently building a pipeline to help me write the "tasks.json" file associated with a Scenario Modeling Hub. The information of the round are available in a README file from which I extract the information.

One of the information I extract and transform is the origin date, extracted and transformed from "text" format to a "YYYY-MM-DD" Date Object format. However, I noticed that if I use the function create_task_id() to create the origin_date task id, I need to transform the date object to character or the function will return an error.

Is that an expected behavior?

Thanks!

origin_date <- as.Date("Sunday November 12, 2023", tryFormats = c("%B %d, %Y", "%A %B %d, %Y"))
str(origin_date)
#>  Date[1:1], format: "2023-11-12"
hubUtils::create_task_id(
  "origin_date", required = origin_date, optional = NULL
)
#> Error in `map()`:
#> ℹ In index: 1.
#> Caused by error in `hubUtils::create_task_id()`:
#> ✖ `required` value must character string of date in valid ISO 8601
#>   format (YYYY-MM-DD).
#> Backtrace:
#>      ▆
#>   1. └─hubUtils::create_task_id("origin_date", required = origin_date, optional = NULL)
#>   2.   └─purrr::walk(...)
#>   3.     └─purrr::map(.x, .f, ..., .progress = .progress)
#>   4.       └─purrr:::map_("list", .x, .f, ..., .progress = .progress)
#>   5.         ├─purrr:::with_indexed_errors(...)
#>   6.         │ └─base::withCallingHandlers(...)
#>   7.         ├─purrr:::call_with_cleanup(...)
#>   8.         └─hubUtils (local) .f(.x[[i]], ...)
#>   9.           └─hubUtils:::check_input(...)
#>  10.             └─cli::cli_abort(...)
#>  11.               └─rlang::abort(...)
hubUtils::create_task_id(
  "origin_date", required = as.character(origin_date), optional = NULL
)
#> $origin_date
#> $origin_date$required
#> [1] "2023-11-12"
#> 
#> $origin_date$optional
#> NULL
#> 
#> 
#> attr(,"class")
#> [1] "task_id" "list"   
#> attr(,"schema_id")
#> [1] "https://raw.githubusercontent.com/Infectious-Disease-Modeling-Hubs/schemas/main/v2.0.0/tasks-schema.json"

Created on 2023-11-01 with reprex v2.0.2

annakrystalli commented 11 months ago

interesting question @LucieContamin . The reason for this behaviour is because I'm reflecting JSON schema checks when I validate the inputs. Any date task ID would be specified in the schema as type: "string" and format: "date".

It is much easier to set up those checks in that order (i.e. first check whether a sting task ID is character in R and then whether it matches the date format, if applicable) than to create a special case for Dates in R. Given we are trying to create JSON (where a string overall data type makes sense) and it's easy enough to use as.character() on a Date, I felt that was ok.

Having said that I could look into it if you feel Dates should be supported.

LucieContamin commented 11 months ago

Thanks for the explanation, that's what I was thinking but was not sure. I am ok with forcing it to character, but in this case, should we maybe update the error message, please? Because currently if you provide a Date in the correct YYYY-MM-DD format, it returns:

#> Caused by error in `hubUtils::create_task_id()`:
#> ✖ `required` value must character string of date in valid ISO 8601
#>   format (YYYY-MM-DD).

It's maybe only me but it took me a little bit of time to understand that the Object format was the issue not the YYYY-MM-DD, so maybe we can add

#> ✖ `required` value must character string of date in valid ISO 8601
#>   format (YYYY-MM-DD), Date object format not accepted.

or something like this?

annakrystalli commented 11 months ago

Yes definitely. Will improve the message.