Repository navigation
shift better naming for negative n #3223
Description
as discussed in inline comments to #3166
now when shift accepts negative integers we can get column names like V1_lag_-1 / V1_lead_-1, they look awkward.
-
My suggestion is to use
V1_shift_1/V1_shift_-1instead. This cannot be made without breaking change to current behavior, unless we introduce new type, justtype="shift". Not much changes to code, and users who prefer to use only new interface (positive to lag, negative for lead) could switch totype="shift". Good to have it for 1.12.0 as we are releasing negative n feature there. -
Additionally on docker tests we are getting - fixed in 76d4a62 - issue was that
match.argwas using shell dependent quoting
Test 1963.8 didn't produce the correct error :
Expected: should be one of "lag"
Observed: 'arg' should be one of “lag”, “lead”
those are environments striped out from many R stuff that is by default active, like methods package.
test(1963.8, shift(DT$x, type = 'some_other_type'),
error = 'should be one of "lag"')- fix invalid test 1963.4,, atomic vectors should not be named
Activity
shift now accept type="shift" for more intuitive give.names=T output, c…
st-pasha commentedon Dec 17, 2018
The problem with a name such as
V1_lag_-1is that it cannot be used as a variable:DT[:, V1_lag_-1]will not work. Backticks may be used as a workaround, but they may not work everywhere, may not be widely known, and are more cumbersome.What can be done is to automatically replace
lag<=>leadupon getting a negativen. This way the produced names will beV1_lag_1andV1_lead_1respectively.MichaelChirico commentedon Dec 17, 2018
jangorecki commentedon Dec 18, 2018
Backticks is not a workaround but standard way to deal with column names like this. From the programmatic point of view it is much better to deal with backticks than condition on your input to shift() to operate its results later on. Consistency is no less important than user friendliness.
mattdowle commentedon Dec 20, 2018
Everyone can be satisfied here. In 32eee2b I kept new
type="shift"and flipped the lead/lag in the names too. That way users can use whichever they prefer.NEWS item now reads :