Repository navigation
shift should accept negative n #1708
Description
Activity
Could make moving-average windowing very simple
Agreed that this looks much better. At the time, I added
type=with more types in mind. Particularlycyclicshift. But that doesn't seem to come up much (on SO). Perhapstype=could be removed altogether.Having a cyclic shift would be interesting, and that would also benefit from having negative
n's. I agree though that it doesn't come up much - I can't think of any real life use cases right now.could you elaborate what you guys have in mind with
cyclicshift?@MichaelChirico I think they just mean that the unused values are shifted around to "fill", like
cyc_shift = function(x, n) c(tail(x,-n), head(x,n)) cyc_shift(LETTERS[1:5], -2) # DEABC lag by 2 cyc_shift(LETTERS[1:5], 2) # CDEAB lead by 2Michael, google for
circshiftfunction in Matlab. That's more or less the functionality I had in mind.Reacted by Michael ChiricoWould still love to have this -- so many times I want to facilitate moving averages like
sapply(-2:2, shift, x = V1)Is there agreement on negative
nmapping totype?
Defaulttypeis"lag", so it make sense to treat negativenastype="lead", while @franknarf1 example ofcyc_shiftpropose the opposite.
Son=-2would be translated ton=2andtype"lead"or"lag"?
@arunsrinivasan what is your preference?I think that having negative
ntranslated toleadwould be very confusing to quite about everyone. This will bring issue/questions on SO and a lot of fuss in general IMO.It will be more consistent to current code, as described above.
Also more conceptually when you compare output oflagandleadyou can observe thanlagadds extra elements in front of vector, whileleadremoves them. And that somehow maps tolag -> +andlead -> -.
No matter what we will decide it will be backward compatible anyway.Yeah, I guess you are right if we want to keep this backward compatible. I was thinking it could be a good thing to deprecate
typealtogether, but that will break too much code.We can just map it change
nargument, so we don't need to support type in C api. Also mention in docs to not usetype. Then after few years should be safe to add warning and after another few to error.@jangorecki
stats::lagalso accepts negative values to get a lead. So, I would use negative values here as well to get a lead (which makes it consistent with the behavior ofstats::lag).Reacted by Jan Goreckii think because it's ambiguous it makes sense to keep so that code can be more readable.
shift(x, 1, type = 'lead')in anyone's code can instantly be understood, I think.Unless we want to go the
NAMESPACE-heavy route & add alias functionsleadandlag.That said, I think most of the use cases involving mixed indices are of the form
-n:nfor whichtypeis irrelevant.If we're going to deprecate anything, maybe the name
shiftin favor of the more universalleadand/orlag(e.g. they're SQL-standard). I thinklag(x, -3:5)is also pretty clear.I don't see anything wrong about name
shift, good thing is that it is not masking other functions, thus better to keep it instead oflag. Eventuallyflagfor fast-lag, butshiftis pretty fine, and flag is already another existing word :)
Lets put it into 1.12.0.Default type is "lag", so it make sense to treat negative n as type="lead", while @franknarf1 example of cyc_shift propose the opposite.
That looks like a typo on my part; sorry for the confusion.
Reacted by Jan GoreckiSO post to update, https://stackoverflow.com/a/52432516
# now ... n0 <- V2 != 0 n0 | shift(n0) | shift(n0, type = 'lead') # should be ... Reduce(`|`, shift(V2 != 0, -1:1))Tangent: This "shift a single logical vector and then collapse by OR or AND" use case is pretty common for me at least. It is essentially a rolling
any/alland can be more done efficiently than via shift-Reduce, I guess. (For OR, find positions where the input is TRUE, subtractnto navigate to rows in output that should be TRUE.)- added a commit that references this issue
on Nov 30, 2018
I think using "type = lag/lead" is a lot of unnecessary typing, and using a negative shift would be much simpler and more intuitive. It would also allow for more nifty multiple shifts going from lead to lag by typing e.g.
shift(vec, -5:5).The "type" argument can still stay and negative shift would simply reverse the meaning of "type".
n = 0should also be accepted and simply return the original vector.