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.

Sign up for GitHub

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

Jump to bottom

endpoints() should return consistently an error when not k > 0 #301

Closed
Eluvias opened this issue Jun 2, 2019 · 2 comments
Closed

endpoints() should return consistently an error when not k > 0 #301

Eluvias opened this issue Jun 2, 2019 · 2 comments
Assignees
@joshuaulrich
Labels
bug
Milestone
0.12.2

Comments

@Eluvias
Copy link

Eluvias commented Jun 2, 2019

endpoints() throws an error when not k > 0 for all cases of on except of the following: years, quarters and months. This is because the error is raised on the cases where the C implementation is used which checks for the values of k.

The case of on = months is an exception as here the C implementation is used but the k value is fixed at 1L.

months = {
        ixmon <- posixltindex$year * 100L + 190000L + posixltindex$mon
        ep <- .Call("endpoints", ixmon, 1L, 1L, addlast, 
            PACKAGE = "xts")
        if (k > 1) ep[seq(1, length(ep), k)] else ep

The cases of on = quarters or on = years are using the R implementation and they don't check for k > 0.

Examples

library(xts)
xx <- as.xts(1:30,
              seq.Date(as.Date(Sys.Date()),
                       by = "day",
                       length.out = 30))

Error is raised when not k > 0

 endpoints(xx, on = "ms", k = -1)
# Error in endpoints(xx, on = "ms", k = -1) : 'k' must be > 0

 endpoints(xx, on = "minutes", k = -1)
# Error in endpoints(xx, on = "minutes", k = -1) : 'k' must be > 0

 endpoints(xx, on = "days", k = -1)
# Error in endpoints(xx, on = "days", k = -1) : 'k' must be > 0

 endpoints(xx, on = "week", k = -1)
# Error in endpoints(xx, on = "week", k = -1) : 'k' must be > 0

Error is not raised when not k > 0

 endpoints(xx, on = "months", k = -1)
> [1]  0 29 30

 endpoints(xx, on = "quarters", k = -1)
> [1]  0 29 30

 endpoints(xx, on = "years", k = -1)
> [1]  0 30
@joshuaulrich joshuaulrich self-assigned this Sep 15, 2020
@joshuaulrich joshuaulrich added this to the 0.12.2 milestone Sep 15, 2020
@joshuaulrich
Copy link
Owner

Thanks for the report!

joshuaulrich added a commit that referenced this issue Sep 15, 2020
@joshuaulrich
Ensure k < 1 errors for every 'on' value
00c6122 
endpoints() was not throwing an error for `k < 1` for `on` of "years",
"quarters", or "months". This is because the only check for `k < 1` was
in the C code called for the other values of `on`.

Thanks to Eluvias for the report.

Fixes #301.
@Eluvias
Copy link
Author

Eluvias commented Sep 28, 2020

Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@joshuaulrich joshuaulrich

Labels
bug
Projects
None yet
Milestone
0.12.2
Development

No branches or pull requests
2 participants
@joshuaulrich @Eluvias