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.

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

circshift!(x, n) and circshift(x, n) shift in different directions #46533

Closed
artemsolod opened this issue Aug 29, 2022 · 2 comments · Fixed by #46759
Closed

circshift!(x, n) and circshift(x, n) shift in different directions #46533

artemsolod opened this issue Aug 29, 2022 · 2 comments · Fixed by #46759
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@artemsolod
Copy link

circshift!(vec, n) rotation direction is inconsistent with copying circshift(vec, n) and mutating destination circshift!(dest, source, n) versions. Judging by #46016 it appears that in-place circshift!(vec, n) was introduced in 1.8 and implemented in #42678

julia> circshift(collect(1:3), 1)
3-element Vector{Int64}:
 3
 1
 2

julia> circshift!(collect(1:3), 1)
3-element Vector{Int64}:
 2
 3
 1

julia> circshift!(collect(1:3), collect(1:3), 1) # with destination and source
3-element Vector{Int64}:
 3
 1
 2
Julia Version 1.8.0
Commit 5544a0fab76 (2022-08-17 13:38 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-13.0.1 (ORCJIT, ivybridge)
@Keno
Copy link
Member

Keno commented Aug 29, 2022

yikes

@fredrikekre fredrikekre added the bug Indicates an unexpected problem or unintended behavior label Sep 14, 2022
fredrikekre added a commit that referenced this issue Sep 14, 2022
This patch fixes the shifting direction for
circshift!(x::AbstractVector, n::Integer), which was opposite the
direction of circshift(x, n) and circshift!(y, x, n). In addition, this
patch fixes the method to also work correctly with offset arrays.
Fixes #46533.
fredrikekre added a commit that referenced this issue Sep 14, 2022
This patch fixes the shifting direction for
circshift!(x::AbstractVector, n::Integer), which was opposite the
direction of circshift(x, n) and circshift!(y, x, n). In addition, this
patch fixes the method to also work correctly with offset arrays.
Fixes #46533.
fredrikekre added a commit that referenced this issue Sep 27, 2022
This patch fixes the shifting direction for
circshift!(x::AbstractVector, n::Integer), which was opposite the
direction of circshift(x, n) and circshift!(y, x, n). In addition, this
patch fixes the method to also work correctly with offset arrays.
Fixes #46533.
fredrikekre added a commit that referenced this issue Sep 27, 2022
This patch fixes the shifting direction for
circshift!(x::AbstractVector, n::Integer), which was opposite the
direction of circshift(x, n) and circshift!(y, x, n). In addition, this
patch fixes the method to also work correctly with offset arrays.
Fixes #46533.
@Keno
Copy link
Member

Keno commented Sep 27, 2022

Thanks for taking care of this @fredrikekre.

KristofferC pushed a commit that referenced this issue Sep 30, 2022
This patch fixes the shifting direction for
circshift!(x::AbstractVector, n::Integer), which was opposite the
direction of circshift(x, n) and circshift!(y, x, n). In addition, this
patch fixes the method to also work correctly with offset arrays.
Fixes #46533.

(cherry picked from commit f1c4d54)
KristofferC pushed a commit that referenced this issue Oct 28, 2022
This patch fixes the shifting direction for
circshift!(x::AbstractVector, n::Integer), which was opposite the
direction of circshift(x, n) and circshift!(y, x, n). In addition, this
patch fixes the method to also work correctly with offset arrays.
Fixes #46533.

(cherry picked from commit f1c4d54)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants