JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.68k stars 5.48k forks source link

Deprecate circshift() and make it lazy #25003

Open nalimilan opened 6 years ago

nalimilan commented 6 years ago

This came up in a Slack discussion about adding lag and lead functions, which are similar to circshift. In many cases, allocating a vector to store the result of circshift is a waste, as a lazy view would be sufficient and as efficient. Looking at uses of circshift on GitHub shows several examples where the result is discarded immediately.

It would make sense to have Iterators.circshift, returning a custom AbstractArray object which would redirect getindex calls to the parent after shifting the index. circshift!(dest, src, shift) would be replaced with copy!(dest, Iterators.circshift(src, shift)).

StefanKarpinski commented 6 years ago

Do we need it in Base at all? Seems like there are no uses, only definitions, which suggest to me that it would make a good stdlib package.

nalimilan commented 6 years ago

Probably, but what package? Something were we'd also put lag and lead?

StefanKarpinski commented 6 years ago

Initially, it could just be stdlib/CircularShifts or something. Could later be rolled into a more general package for utility views into arrays.

nalimilan commented 6 years ago

We could try to find a slightly more general name from the start to avoid renaming it if possible. lag and lead are not "circular" for example. LazyArrays? ArrayTools?

tknopp commented 6 years ago

For me this seem to belong to a module that groups: Array slicing, Array reshaping, Array transpose. Its not super necessary that circshift sits in base. But discoverability is IMHO pretty important.

nalimilan commented 6 years ago

Slicing, reshaping and transpose are really basic features, I'm not sure we want to move them to stdlib...

bramtayl commented 6 years ago

If we're talking about slicing JuliennedArrays.jl does slicing much faster than mapslices in Base and also is much more flexible.

StefanKarpinski commented 6 years ago

JuliennedArrays is the best-named package.

bramtayl commented 6 years ago

be still my heart

JeffBezanson commented 6 years ago

We can probably punt on this for 1.0.

nalimilan commented 6 years ago

That's clearly not a big issue, but if we want to deprecate the functions something will have to be done before 1.0 (either a deprecation or moving them to stdlib).

StefanKarpinski commented 6 years ago

This can be a vestigial API at some point in the future when a lovely, complete lazy package exists.

piever commented 6 years ago

Implemented in the ShiftedArrays package, together with lag and lead. The two new AbstractArray types are ShiftedArray (meaning: you get missing if you are out of bounds) for lag and lead and CircShiftedArray (meaning, you restart from the other extreme) for circshift.

StefanKarpinski commented 6 years ago

I guess the question now is what do we want the relationship between this Base function and the ShiftedArrays package to be? Should we consider deprecating it entirely and pointing people at the package instead? Or should we retain this as a way of doing eager array shifting?