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

Incorrect conditions in .validate_dates (internal functions) #46

Open
Nblws opened this issue Jan 20, 2023 · 2 comments
Open

Incorrect conditions in .validate_dates (internal functions) #46

Nblws opened this issue Jan 20, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@Nblws
Copy link

Nblws commented Jan 20, 2023

At the moment it is not possible to get values from first and last dates as conditions in .validate_dates are strict :

......................
# no older than past date
cond2 <- xmin > past

# no later then present date
cond3 <- xmax < present
......................

At least for the condition on past date, I believe this should be greater or equal.

Best,
Victor

@kauedesousa
Copy link
Member

Can you make an example please?

adamhsparks added a commit that referenced this issue Jan 21, 2023
fixes incorrect conditions when checking dates for `present`
@adamhsparks
Copy link
Member

adamhsparks commented Jan 21, 2023

I think that @Nblws is correct here, @kauedesousa. See my reprex below. If the very first available date and very last available dates are selected then you end up with invalid dates.

availability = c("1981-01-01", "0")
x = c("1981-01-01", format(Sys.Date() - 45, "%Y-%m-%d"))

xmin <- as.Date(x[1], format = "%Y-%m-%d")

xmax <- as.Date(x[2], format = "%Y-%m-%d")

# the first day from which the dataset is available
past <- as.Date(availability[1], origin = "1970-01-01")

# the most recent date from which the dataset is available
present <- availability[2]

# generally it takes 45 days to update
if (present == "0") {
  present <- Sys.Date() - 45
  present <- format(present,  "%Y-%m-%d")
}

# last given date should be higher than first
cond1 <- as.integer(xmax - xmin) > 1

# no older than past date
cond2 <- xmin > past

# no later then present date
cond3 <- xmax < present

cond1
#> [1] TRUE

cond2
#> [1] FALSE

cond3
#> [1] FALSE

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

If you change as suggested you end up passing.

availability = c("1981-01-01", "0")
x = c("1981-01-01", format(Sys.Date() - 45, "%Y-%m-%d"))

xmin <- as.Date(x[1], format = "%Y-%m-%d")

xmax <- as.Date(x[2], format = "%Y-%m-%d")

# the first day from which the dataset is available
past <- as.Date(availability[1], origin = "1970-01-01")

# the most recent date from which the dataset is available
present <- availability[2]

# generally it takes 45 days to update
if (present == "0") {
  present <- Sys.Date() - 45
  present <- format(present,  "%Y-%m-%d")
}

# last given date should be higher than first
cond1 <- as.integer(xmax - xmin) > 1

# no older than past date
cond2 <- xmin >= past

# no later then present date
cond3 <- xmax <= present

cond1
#> [1] TRUE

cond2
#> [1] TRUE

cond3
#> [1] TRUE

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

Looking at the devel branch, the past is tested correctly, but present would still fail. This hasn't made it over to the master branch yet, so I'll fix the present test there too and you can check and merge if it's acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants