Skip to content

shift no longer works on matrix type under 1.14.3 #5287

Closed
#5462
Closed
shift no longer works on matrix type under 1.14.3#5287
#5462
Assignees
jangorecki
Milestone
1.15.0

Description

@ethanbsmith
ethanbsmith
opened on Dec 12, 2021

shift used to work on matrix columns, but now generates and error. i dont see anything in the NEWS. is this intentional as its a breaking change?

works in 1.14.0

sessionInfo()
R version 4.1.2 (2021-11-01)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19044)

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] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
[1] compiler_4.1.2    tools_4.1.2       data.table_1.14.0 checkpoint_1.0.0 
> data.table::shift(matrix(1:10, ncol = 1))
 [1] NA  1  2  3  4  5  6  7  8  9

not working under 1.14.3

 sessionInfo()
R version 4.1.2 (2021-11-01)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19044)

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] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
[1] compiler_4.1.2    tools_4.1.2       data.table_1.14.3 checkpoint_1.0.0 
> data.table::shift(matrix(1:10, ncol = 1))
Error in data.table::shift(matrix(1:10, ncol = 1)) : 
  'as' must not be matrix or array

Activity

  1. ethanbsmith
    changed the title [-]1.14.3 `shift` no longer works on matrix type[/-] [+]`shift` no longer works on matrix type under 1.14.3[/+] on Dec 12, 2021

  2. ben-schwen commented on Dec 12, 2021

    @ben-schwen
    ben-schwen
    on Dec 12, 2021
    Member

    Introduced in #5189. It's erroring because coerceAs cannot handle matrix/arrays.
    Same error ofc also appears for nafill(matrix(c(NA, 1:9), ncol = 1), fill=0) which also happened to "work" before #4491.

    According to docs shift/nafill never supported matrix but happened to work because matrices/array are R internally represented via one-dimensonial arrays with attributes.

    @jangorecki are there reasons why coerceAs(x, as, copy) doesn't allow matrices/arrays for arguments x and as?

  3. jangorecki commented on Dec 13, 2021

    @jangorecki
    jangorecki
    on Dec 13, 2021
    Member

    There reason is that there was no such requirement when it was implemented.
    https://rdatatable.gitlab.io/data.table/library/data.table/html/shift.html does not mention matrix, but "A vector, list, data.frame or data.table" only, as Ben pointed out.

  4. MichaelChirico commented on Dec 19, 2021

    @MichaelChirico
    MichaelChirico
    on Dec 19, 2021
    Member

    the fix in this case is very simple:

    data.table::shift(c(matrix(1:10, ncol = 1)))

    The earlier behavior on multi-column matrices is IMO quite undesirable, so I'm happy to break it in this case. Though we could improve the error message by catching it earlier.

  5. ethanbsmith commented on Dec 19, 2021

    @ethanbsmith
    ethanbsmith
    on Dec 19, 2021 via email
    ContributorAuthor
  6. mattdowle
    added this to the 1.14.3 milestone on Aug 18, 2022

  7. jangorecki commented on Sep 14, 2022

    @jangorecki
    jangorecki
    on Sep 14, 2022
    Member

    Though we could improve the error message by catching it earlier.

    Was about to do it, but realized I will just copy error message from coerceAs into nafill and shift. IMO it's better to catch it there, in single a place.

  8. jangorecki
    modified the milestones: 1.14.9, 1.15.0 on Oct 29, 2023
  9. jangorecki
    self-assigned this
    on Dec 8, 2023
  10. jangorecki
    closed this as completedin #5462on Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions