NOAA-FIMS / FIMS

The repository for development of FIMS
https://noaa-fims.github.io/FIMS/
GNU General Public License v3.0
12 stars 8 forks source link

[Bug]: `type.convert()` is reformatting date to character without leading zeros in data_mile1 #639

Open kellijohnson-NOAA opened 1 week ago

kellijohnson-NOAA commented 1 week ago

Describe the bug

Here https://github.com/NOAA-FIMS/FIMS/blob/0296e41c9834991a62cd237d2387dca687bb6931/data-raw/data_mile1.R#L152 in data-raw, the script that creates data_mile1 the dates are being reformatted as character strings rather than date objects. Thus, when n_years() attempts to calculate the number of years in the data it thinks that "9" is the maximum year rather than "30". This line of code has not changed for two years and used to work so I am not sure what is going on.

I am wondering if we can just remove the call to type.convert() and just have rbind()?

To Reproduce

source(textConnection(readLines("data-raw/data_mile1.R")[1:151]))
type.convert(rbind(landings_data, index_data), as.is = TRUE) |>
  dplyr::pull(dateend) |>
  class()
[1] "character"
> rbind(landings_data, index_data) |> dplyr::pull(dateend) |> class()          
[1] "Date"

 type.convert(rbind(landings_data, index_data), as.is = TRUE) |>
  dplyr::pull(dateend) |> 
  max()  
[1] "9-12-31"
 rbind(landings_data, index_data) |> dplyr::pull(dateend) |> max()  
[1] "0030-12-31"

Screenshots

No response

Which OS are you seeing the problem on?

Windows

Which browser are you seeing the problem on?

No response

Which version of FIMS are you seeing the problem on?

'0.2.0.0' since #37e659c

Additional Context

No response

kellijohnson-NOAA commented 1 week ago

Removing type.convert() is causing problems when the data are written to a csv and read back in. Maybe we should coerce datestart and dateend using as.Date() on the back end in FIMSFrame() and keep type.convert()?

iantaylor-NOAA commented 1 week ago

I'm a fan of type.convert() so support keeping while converting those columns to dates at whatever point makes the most sense.