Bug fix for gmin and blank strings

[khotilov]

Closes #4848

Before the fix:

> library(data.table)
> d = data.table(id=c(1,1), a=c("","a"))
> d[, min(a), id]
      id     V1
   <num> <char>
1:     1      a
> data.table:::gmin(c("a", ""))
[1] ""
> data.table:::gmin(c("", "a"))
[1] "a"

After the fix:

> d = data.table(id=c(1,1), a=c("","a"))
> d[, min(a), id]
      id     V1
   <num> <char>
1:     1
> data.table:::gmin(c("a", ""))
[1] ""
> data.table:::gmin(c("", "a"))
[1] ""

[MichaelChirico]

thanks! could you please file an Issue to go with this? please include in the issue the base behavior for this case as well

…

On Tue, Dec 15, 2020, 10:31 AM Vadim Khotilovich ***@***.***> wrote: Before the fix: > library(data.table)> d = data.table(id=c(1,1), a=c("","a"))> d[, min(a), id] id V1 <num> <char>1: 1 a> data.table:::gmin(c("a", "")) [1] ""> data.table:::gmin(c("", "a")) [1] "a" After the fix: > d = data.table(id=c(1,1), a=c("","a"))> d[, min(a), id] id V1 <num> <char>1: 1> data.table:::gmin(c("a", "")) [1] ""> data.table:::gmin(c("", "a")) [1] "" ------------------------------ You can view, comment on, or merge this pull request online at: #4847 Commit Summary - Fix gmin bug: result depended on blank strings position File Changes - *M* inst/tests/tests.Rraw <https://github.com/Rdatatable/data.table/pull/4847/files#diff-e3243f3780ce7d303c3317f73945310bfc37e45d193568246246aca20e3270ae> (7) - *M* src/gsumm.c <https://github.com/Rdatatable/data.table/pull/4847/files#diff-6222d4a3d04343943427c4cbfe3c92d0095569384ab7428b1d5f4c18fdc48d45> (10) Patch Links: - https://github.com/Rdatatable/data.table/pull/4847.patch - https://github.com/Rdatatable/data.table/pull/4847.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#4847>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2BA5LWVO7D5SVQFSOCEB3SU56M7ANCNFSM4U4QKRHA> .

[codecov]

Codecov Report

Merging #4847 (ad08153) into master (03dff91) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4847   +/-   ##
=======================================
  Coverage   99.47%   99.47%           
=======================================
  Files          75       75           
  Lines       14775    14776    +1     
=======================================
+ Hits        14697    14698    +1     
  Misses         78       78           

Impacted Files	Coverage Δ
src/gsumm.c	99.90% <100.00%> (ø)
src/init.c	100.00% <100.00%> (ø)

---

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 03dff91...ad08153. Read the comment docs.

[khotilov]

@MichaelChirico here's an issue #4848

[MichaelChirico]

is upd for update?

[khotilov]

Yep, changed to updated.

[mattdowle]

Thanks @khotilov. I've invited you to be project member so that, among other things, you can create branches directly in the main project in future. The invite is a button that appears in your GitHub profile or projects page which you need to click to accept. Also added you to DESCRIPTION in this PR. Thanks again, and welcome!