.Last.updated value after :=, closes #1885

[jangorecki]

Closes #1885
Now resolved: but adds extra overhead of sum(!is.na(irows)), so should not be merged. Most likely changes needs to be pushed to C code where the actual updated number of rows is established. Not doing it now to avoid merge conflicts with rbindlist rework branch

[codecov]

Codecov Report

Merging #3460 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3460      +/-   ##
==========================================
+ Coverage   95.17%   95.17%   +<.01%     
==========================================
  Files          65       65              
  Lines       12298    12302       +4     
==========================================
+ Hits        11705    11709       +4     
  Misses        593      593

Impacted Files	Coverage Δ
R/data.table.R	97.48% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a355cf...36b6eef. Read the comment docs.

[codecov]

Codecov Report

Merging #3460 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3460      +/-   ##
==========================================
+ Coverage   97.01%   97.01%   +<.01%     
==========================================
  Files          66       66              
  Lines       12475    12484       +9     
==========================================
+ Hits        12102    12111       +9     
  Misses        373      373

Impacted Files	Coverage Δ
src/init.c	100% <100%> (ø)	⬆️
R/data.table.R	97.64% <100%> (ø)	⬆️
src/assign.c	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c489ebb...f390515. Read the comment docs.

[mattdowle]

Good idea. Would be nice to have this.
The only thing is that packages aren't allowed to write to .GlobalEnv without user's consent (CRAN policy). It could be optional, default off, turned on by user. But then what if a package wished to use it inside that package? data.table could have an object which it exports though: .Last.updated, a length-1 integer vector. Packages are allowed to change variables they export (like assignInNamespace does). Then users can access the value via the usual search() path or explicitly using data.table::.Last.updated.
Is .Last.updated the best name? Maybe it should have nrows or similar in the name somehow to convey it's a number of rows.

[jangorecki]

I pushed new version, using C and writing variable in data.table namespace.
The issue is that I am getting failure of one of the tests when running package check, but I am not able to reproduce it interactively. I have no clue about the reason for that.

Running test id 2028.14      Test 2028.14 ran without errors but failed check th
at x equals y:
> x = .Last.nrow 
First 6 of 1 (type 'integer'): [1] 1
> y = 4L 
First 6 of 1 (type 'integer'): [1] 4
Mean relative difference: 3

[MichaelChirico]

No manual entry?

[mattdowle]

Good catch. Could be added to ?:= with alias at the top of that .Rd so ?.Last.updated works.

[jangorecki]

I remember I made manual, but it seems I was just using git add -u so new files has not been added. Strangely, I must have deleted that branch locally already.

[mattdowle]

I guess this means that R CMD check requires exported functions to be documented but not exported variables. That's odd if so. I can't think why it shouldn't check exported variables too. Maybe we have other undocumented exported variables then.

[MichaelChirico]

This means the initial value (even if data.table is not loaded) is 0L, is that preferable to NA_integer_?

library(data.table)
.Last.updated
# [1] 0
DT = data.table(a = 1)
DT[FALSE, a := 2]
.Last.updated
# [1] 0

Conceptually, I think .Last.updated is different in the two cases?

[MichaelChirico]

Is there any benefit to having a reset_last_value function as well, or it should be taken as immutable? I note it already seems like we can't overwrite its value manually...

[MichaelChirico]

We need to add the same note as #2837 alongside the .Last.updated man page, wherever it ends up:

DT = data.table(a = 1L)
DT[c(1, 1), a := 2:3]
DT[]
#    a
# 1: 3
.Last.updated
# [1] 2

[jangorecki]

I don't think we need to worry about consistency on first time use, before any update. And yes, good point about duplicates on assignment, will add together with manual