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

first and last return all elements when n = 0 #350

Closed
ethanbsmith opened this issue Apr 8, 2021 · 7 comments
Closed

first and last return all elements when n = 0 #350

ethanbsmith opened this issue Apr 8, 2021 · 7 comments
Labels
bug
Milestone
0.12.2

Comments

@ethanbsmith
Copy link
Contributor

ethanbsmith commented Apr 8, 2021 •
edited
Loading

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
Copy link
Owner

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

@braverock
Copy link
Contributor

I guess n=0 could return NA or NULL

@joshuaulrich
Copy link
Owner

joshuaulrich commented Apr 8, 2021 via email

@braverock
Copy link
Contributor

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
Copy link
Contributor Author

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 Rdatatable/data.table#4053

@ethanbsmith
Copy link
Contributor Author

i've taken a stab at the unit test and changes to first.r
master...ethanbsmith:fix_350

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

@joshuaulrich
Copy link
Owner

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 joshuaulrich added this to the 0.12.2 milestone Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
0.12.2
Development

No branches or pull requests
3 participants
@joshuaulrich @braverock @ethanbsmith