Restore behaviour from 0.10-2 to .xts()

[TomAndrews]

Add unit tests to document the behaviour in 0.10-2 and fix the failures introduced in 0.11. Fixes #249.

[TomAndrews]

Hmm the checks pass locally on R 3.5.1. I'll investigate.

[TomAndrews]

I'm stumped, I can't replicate the test failures on any of the 3 machines I have easy access to. Any hints would be appreciated.

[joshuaulrich]

Since the failing test is for the tzone attribute, my guess is that the cause is an unset/different TZ environment variable. What is Sys.getenv("TZ") on your local machines? You might need to add a .setUp() or something to your tests to Sys.setenv(TZ = "UTC") or similar.

[TomAndrews]

Yes, explicitly setting TZ in .setUp fixed it. Thanks!

[TomAndrews]

I've added a commit to alter the two checkXts* functions so that they check the two accessor functions and the attribute on the index.

Testing in v0.10-2 (see my branch here) means that the 3 TZ checks are now always consistent - you don't ever see the differing values of .indexTZ and tzone.

It seems that for timezones in v0.10-2 specifying tzone on the xts object takes precedence over setting tzone on the index of the xts. .indexTZ always seems to be ignored, which is what is causing the last odd looking test case. I have updated the tests to reflect all this.

As it stands this pull request reinstates the behaviour from v0.10-2; the updated tests still pass with my original fixes. But you might want to do something different with the C code to set .indexTZ: it seems a bit pointless to pass it in and set it if it's never actually used.

Do let me know if you'd like any other changes.

[TomAndrews]

In fact, on closer inspection, my changes to the handling of .indexTZ are not required to make the unit tests pass as long as you use the accessor functions and don't look at the attributes directly.

I can squash this down if you're happy.

[joshuaulrich]

But you might want to do something different with the C code to set .indexTZ: it seems a bit pointless to pass it in and set it if it's never actually used.

Thanks for noticing this and mentioning it. The plan is to deprecate .indexTZ and .indexClass (in favor of tzone and tclass on the index), so I'm not concerned that it's not currently being used.

This looks good to me now. I would appreciate a cleaned-up history (squash or rebase, depending on what you think tells the story better), especially:

• adding to the commit message(s) any relevant parts of this conversation, and
• See #249. and/or Fixes #249. on a line at the end of the commit message, so the commit appears in the issue tracker.

[joshuaulrich]

Great work @TomAndrews! Thank you very much for your contributions!