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

Problem with indexing after change of indexing type #311

Closed
dppalomar opened this issue Oct 4, 2019 · 5 comments
Closed

Problem with indexing after change of indexing type #311

dppalomar opened this issue Oct 4, 2019 · 5 comments
Milestone
0.13.2

Comments

@dppalomar
Copy link

dppalomar commented Oct 4, 2019 •
edited by joshuaulrich
Loading

Description

One of the great advantages of xts is its flexibility in indexing the observations with dates and so. However, I found a strange behaviour after I change the indexing type of the xts which IMHO should not happen.

Minimal, reproducible example

Let's start with a toy xts and some indexing which works fine:

library(xts)

tvec <- c("2014-01-01 19:00:00", "2014-01-02 19:00:00", "2014-01-05 19:00:00")
test <- xts(rnorm(3), order.by = as.POSIXct(tvec, tz = "Asia/Hong_Kong"))
tclass(test)  # "POSIXct" "POSIXt" 
test["2014-01-01"]  # 2014-01-01 19:00:00 0.8174548
test[index(test["2014-01-01"])]  # 2014-01-01 19:00:00 0.8174548

Now, let's change the index type from POSIXct to Date and we will get the weird behaviour:

test2 <- convertIndex(test, "Date")
tclass(test2)  # "Date"
test2["2014-01-01"]  # 2014-01-01 0.8174548
test2[index(test2["2014-01-01"])]  # What???!!!  Empty!!!

Obviously, had I created the xts originally with Date, then there would be no problem:

test3 <- xts(rnorm(3), order.by = as.Date(tvec))
tclass(test3)
test3["2014-01-01"]
index(test3["2014-01-01"])
test3[index(test3["2014-01-01"])]

Therefore, the problem comes from the change of type of index, but I don't think this should happen...

Session Info

> sessionInfo()
R version 3.6.1 (2019-07-05)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Mojave 10.14.6

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] portfolioBacktest_0.2.0.9000 testthat_2.1.1               xts_0.11-2                   zoo_1.8-6                   

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.2                 lattice_0.20-38            prettyunits_1.0.2          ps_1.3.0                  
 [5] snow_0.4-3                 assertthat_0.2.1           rprojroot_1.3-2            digest_0.6.21             
 [9] foreach_1.4.7              mime_0.7                   R6_2.4.0                   backports_1.1.4           
[13] evaluate_0.14              ggplot2_3.2.1              pillar_1.4.2               rlang_0.4.0               
[17] lazyeval_0.2.2             curl_4.2                   rstudioapi_0.10            callr_3.3.1               
[21] TTR_0.23-5                 R.utils_2.9.0              R.oo_1.22.0                DT_0.7                    
[25] desc_1.2.0                 devtools_2.2.0             stringr_1.4.0              htmlwidgets_1.3           
[29] munsell_0.5.0              shiny_1.3.2                compiler_3.6.1             httpuv_1.5.1              
[33] pkgconfig_2.0.3            pkgbuild_1.0.5             htmltools_0.3.6            doSNOW_1.0.18             
[37] tidyselect_0.2.5           gridExtra_2.3              tibble_2.1.3               roxygen2_6.1.1            
[41] quadprog_1.5-7             codetools_0.2-16           later_0.8.0                crayon_1.3.4              
[45] dplyr_0.8.1                withr_2.1.2                R.methodsS3_1.7.1          commonmark_1.7            
[49] grid_3.6.1                 xtable_1.8-4               gtable_0.3.0               magrittr_1.5              
[53] scales_1.0.0               PerformanceAnalytics_1.5.3 quantmod_0.4-15            cli_1.1.0                 
[57] stringi_1.4.3              promises_1.0.1             fs_1.3.1                   remotes_2.1.0             
[61] xml2_1.2.0                 ellipsis_0.3.0             rcmdcheck_1.3.3.9000       iterators_1.0.12          
[65] tools_3.6.1                glue_1.3.1.9000            purrr_0.3.2                crosstalk_1.0.0           
[69] processx_3.4.1             pkgload_1.0.2              parallel_3.6.1             colorspace_1.4-1          
[73] xopen_1.0.0                sessioninfo_1.1.1          memoise_1.1.0              usethis_1.5.0
@joshuaulrich
Copy link
Owner

Thanks for the report! I see that you're using xts_0.11-2. Can you please try the current development version 0.12.0 and report back?

@dppalomar
Copy link
Author

Joshua, I tried as you said

other attached packages:
[1] xts_0.12-0 zoo_1.8-6

but still the same problem...

> test2 <- convertIndex(test, "Date")
> tclass(test2)
[1] "Date"
> test2["2014-01-01"]
                [,1]
2014-01-01 0.5057137
> index(test2["2014-01-01"])
[1] "2014-01-01"
> test2[index(test2["2014-01-01"])]  # What???!!!  Empty!!!
     [,1]

@joshuaulrich
Copy link
Owner

Thanks for checking! I investigated and found the cause is that convertIndex() doesn't actually change the underlying index values. convertIndex() is a simple wrapper around xts:::`tclass<-.xts`, which also does not change the underlying values. So the index values are equal, even though the index attributes are different.

all.equal(.index(test), .index(test2), check.attributes=FALSE)
# [1] TRUE

We've discussed internally in the past whether or not changing the tclass and/or tzone attribute should change the underlying index. As this behavior shows, we've historically decided that the underlying index values should not change. Issues like your seem to happen often enough that perhaps we should reconsider.

@dppalomar
Copy link
Author

Joshua, indeed that's what I suspected. Thanks for looking into it.

IMHO, I think this choice is very misleading and prone to errors. I don't see the point of changing the type of index without actually changing the indices.

So if you ask me, I would either totally change the indices or deprecate this wrapper. But that's my opinion. Thanks!

@joshuaulrich joshuaulrich mentioned this issue Jul 15, 2023
Closed
@joshuaulrich
Copy link
Owner

I'm going to make this change and do reverse dependency checks to see what, if anything, breaks. I think the only time the actual underlying index values would change is when the 'tclass' changes from one with a timezone to one without a timezone.

Part of the change will be a warning any time the 'tclass' changes from one with a timezone to one without a timezone (with an option to always suppress the warning). This should help users identify bugs and hopefully cause failing reverse dependency checks if other package authors have done it.

@joshuaulrich joshuaulrich added this to the 0.13.2 milestone Nov 17, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Nov 24, 2024
(math/R-xts) Updated 0.13.1 to 0.14.1
3e167f0 
# xts 0.14.x (202x-xx-xx)

* `plot.xts()` now renders all panels when 'x' has more than 8 columns and
  `multi.panel = TRUE`. Columns 9 and later didn't render because the default
  of `plot.xts()` is 'col = 1:8'. Thanks to Ethan Smith for the report and
  patch.
  ([#423](joshuaulrich/xts#423))
  ([#424](joshuaulrich/xts#424))

* `plot.xts()` no longer errors when 'ylim' is constant and negative. Thanks
  to Ethan Smith for the report.
  ([#422](joshuaulrich/xts#422))

* Do not use `SET_TYPEOF()` in C because it is not part of the public R API.

* `merge.xts()` no longer converts 'x' or 'y' from double to integer in the C
  code when they are not used in the result. This avoids an unnecessary and
  confusing warning. Thanks to Jeff Ryan for the report.

# xts 0.14.0 (2024-06-05)

* `addEventLines()` and `addLegend()` now draw on multiple panels when `on` is
  a vector. Thanks to Ethan Smith for the report.
  ([#420](joshuaulrich/xts#420))

* Replace `SET_TYPEOF()` in merge.c because it will error when it tries to
  convert a REAL to an INTEGER. Thanks to Kurt Hornik for the report!
  ([#419](joshuaulrich/xts#419))

* Fix crash when 'j' is not an integer and in [0, 1) (e.g. `j = 0.1`). Also
  throw a warning when 'j' is not an integer.
  ([#413](joshuaulrich/xts#413))
  ([#415](joshuaulrich/xts#415))

* Fix plot header when `layout()` is used to draw multiple plots on a single
  device. Thanks to Dirk Eddelbuettel for the report and testing!
  ([#412](joshuaulrich/xts#412))

* Fix plot legend location when the y-axis is log scale.
  ([#407](joshuaulrich/xts#407))

# xts 0.13.2 (2024-01-21)

* Print a message when `period.apply()` is called with `FUN = mean` because it
  calculates the mean for each column, not all the data in the subset like it
  does for all other functions. The message says to use `FUN = colMeans` for
  current behavior and `FUN = function(x) mean(x)` to calculate the mean for
  all the data. This information is also included in the help files. The option
  `xts.message.period.apply.mean = FALSE` suppresses the message.
  ([#124](joshuaulrich/xts#124))

* Fix error when `print.xts()` is called 'quote' or 'right' arguments.
  ([#401](joshuaulrich/xts#401))

* Fix `addPolygon()` so it renders when `observation.based = TRUE`.
  ([#403](joshuaulrich/xts#403))

* Print trailing zeros for index value with fractional seconds, so every index
  value has the same number of characters.
  ([#404](joshuaulrich/xts#404))

* Add ability to log scale the y-axis in `plot.xts()`.
  ([#103](joshuaulrich/xts#103))

* Actually change the underlying index values when 'tclass' is changed from a
  class with a timezone (e.g. POSIXct) to one without a timezone (e.g. Date).
  Add a warning when this happens, with a global option to always suppress the
  warning.
  ([#311](joshuaulrich/xts#311)).

* Significantly refactor the internals of `plot.xts()`.
  ([#408](joshuaulrich/xts#408))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.13.2
Development

No branches or pull requests
2 participants
@joshuaulrich @dppalomar