first and last return all elements when n = 0

ethanbsmith commented 3 years ago

Description

i'm not sure if this is a bug or intended behavior. its not what I expected if this is not a bug, could you help me understand why not so we can possibly enhance the docs

thx

Expected behavior

when n = 0 i expect an empty set top be returned

Minimal, reproducible example

identical(integer(), xts::first(1:5,0))
identical(integer(), xts::last(1:5,0))

Session Info

R version 4.0.4 (2021-02-15)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19042)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252    LC_MONETARY=English_United States.1252 LC_NUMERIC=C                           LC_TIME=English_United States.1252    

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

other attached packages:
 [1] matrixStats_0.58.0 rollRegres_0.1.3   rio_0.5.16         rvest_0.3.6        xml2_1.3.2         data.table_1.14.0  curl_4.3           quantmod_0.4.18    TTR_0.24.2         xts_0.12.1         zoo_1.8-8          doParallel_1.0.16 
[13] iterators_1.0.13   foreach_1.5.1      plotrix_3.8-1      checkpoint_0.4.10 

loaded via a namespace (and not attached):
 [1] zip_2.1.1        Rcpp_1.0.6       compiler_4.0.4   pillar_1.5.0     cellranger_1.1.0 forcats_0.5.1    tools_4.0.4      bit_4.0.4        checkmate_2.0.0  jsonlite_1.7.2   lifecycle_1.0.0  tibble_3.1.0     lattice_0.20-41 
[14] pkgconfig_2.0.3  rlang_0.4.10     openxlsx_4.2.3   haven_2.3.1      stringr_1.4.0    httr_1.4.2       vctrs_0.3.6      hms_1.0.0        bit64_4.0.5      grid_4.0.4       R6_2.5.0         fansi_0.4.2      readxl_1.3.1    
[27] foreign_0.8-81   selectr_0.4-2    magrittr_2.0.1   backports_1.2.1  codetools_0.2-18 ellipsis_0.3.1   utf8_1.1.4       stringi_1.5.3    crayon_1.4.1

joshuaulrich commented 3 years ago

Thanks for the report. I consider this a bug. Do you have thoughts about how to fix?

braverock commented 3 years ago

I guess n=0 could return NA or NULL

joshuaulrich commented 3 years ago

My prior is to return the same as x[0,]. I could be convinced otherwise...

braverock commented 3 years ago

My prior is to return the same as x[0,]. I could be convinced otherwise...

I think that's a good solution. So no error, but no data other than column names.

ethanbsmith commented 3 years ago

agree with x[0,] as output

I haven't looked at the code yet, but will do so this weekend, see if i can come up with a PR or at least suggestions

also, I'd like to loop in @jangorecki and the data.table folks as they have special handling for overiding these functions https://github.com/Rdatatable/data.table/issues/4053

ethanbsmith commented 3 years ago

i've taken a stab at the unit test and changes to first.r https://github.com/joshuaulrich/xts/compare/master...ethanbsmith:fix_350

could you take a look and give any feedback before i start on last.r

joshuaulrich commented 1 year ago

When x is an xts object, data.table::first() and data.table::last() call xts::first() and xts::last(), respectively. data.table will also call the xts functions if x isn't an xts object, but the xts package has been attached.

@ethanbsmith your patch looked good to me, so I made the necessary changes to last.R. Thanks a ton for writing unit tests for both!

joshuaulrich / xts