allow for December to be 12

[hasandiwan]

Please review the contributing guide before submitting your pull request. Please pay special attention to the pull request and commit message sections. Thanks for your contribution and interest in the project!

I wasn't able to figure out how to add/execute an automated tests, but have verified by hand that it works.

[joshuaulrich]

Hi Hasan, can you help me understand what you're trying to accomplish? Your change implies that you want to reassign the value of x to 11 if the user passes in a scalar value of 12 for x. That doesn't make sense to me.

The ".index*" functions generally expect x to be an xts object, or at least an object with "index" and "tzone" attributes.

Also, your change will throw a warning on CRAN if x is longer than one element.

R$ x <- .xts(1:5, 1:5, tzone = "UTC")
R$ .indexmon(x)
[1] 0 0 0 0 0
Warning message:
In if (x == 12) x <- 11 :
  the condition has length > 1 and only the first element will be used

Hmm, and it throws an error if the coredata value in the xts object is 12.

R$ .indexmon(xts(12, as.Date("2021-01-01")))
Error in class(xx) <- cl : attempt to set an attribute on NULL

...because it overwrites the xts object with the scalar value 12, which means .index(x) and tzone(x) both return NULL and creates this call .POSIXct(NULL, tz = NULL).

So I need to understand what you were trying to do. Then we can discuss your idea and determine whether or not it makes sense for xts. Then we can discuss how to implement it.

[hasandiwan]

My thought is that December is the 12th month of the year and is conventionally written as "12". xts seems to want it to be 11, which is a little odd to me. -- H

…

On Fri, 1 Jan 2021 at 08:44, Joshua Ulrich ***@***.***> wrote: Hi Hasan, can you help me understand what you're trying to accomplish? Your change implies that you want to reassign the value of x to 11 if the user passes in a scalar value of 12 for x. That doesn't make sense to me. The ".index*" functions generally expect x to be an xts object, or at least an object with "index" and "tzone" attributes. Also, your change will throw a warning on CRAN if x is longer than one element. R$ x <- .xts(1:5, 1:5, tzone = "UTC")R$ .indexmon(x) [1] 0 0 0 0 0Warning message:In if (x == 12) x <- 11 : the condition has length > 1 and only the first element will be used Hmm, and it throws an error if the coredata value in the xts object is 12. R$ .indexmon(xts(12, as.Date("2021-01-01")))Error in class(xx) <- cl : attempt to set an attribute on NULL ...because it overwrites the xts object with the scalar value 12, which means .index(x) and tzone(x) both return NULL and creates this call .POSIXct(NULL, tz = NULL). So I need to understand what you were trying to do. Then we can discuss your idea and determine whether or not it makes sense for xts. Then we can discuss how to implement it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#344 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKSFUTIWYI7SXXESISKZ5ODSXX3WNANCNFSM4VITQZJQ> .

-- -- H Feel free to respond encrypted using https://sks-keyservers.net/pks/lookup?op=get&search=0xFEBAD7FFD041BBA1

[joshuaulrich]

Yeah, I understand that it's a bit surprising. The rationale is that it follows the convention of R's POSIXlt structure, which is an international standard and is also consistent with the underlying C tm struct. If you're interested, here's some background on the tm struct.

All that said, I don't think we can change this now. It would absolutely break existing code, so I'm going to close this without merging. I'll open an issue with some possible alternatives. Then I'll poke others to get their feedback.