Postpone the Non-API Problem.

[SebKrantz]

In case you want to postpone your solution to #6180. This is all I can offer for now. Runs perfectly without error or notes on all plattforms and would immediately get CRAN off your back and allow you (us) to think about how to properly solve the non-API issues (e.g., full blown ALTREP overhaul a la @aitap + new character match or something else). Seems to me the proper solution is not going to be here by tomorrow. If CRAN is not pushing you then of course you can ignore this. They were not responding anymore to {collapse} submissions with non-API calls.

[TysonStanley]

Thanks! They mentioned on the latest patch submission that was accepted that we need this fixed so this is great timing.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.61%. Comparing base (96e89fa) to head (9cecde1).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6640      +/-   ##
==========================================
- Coverage   98.61%   98.61%   -0.01%     
==========================================
  Files          79       79              
  Lines       14543    14521      -22     
==========================================
- Hits        14342    14320      -22     
  Misses        201      201              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[SebKrantz]

Should have only minor performance implications. One thing I could add to compensate/make data.table faster is to replace TRULEN (#define TRULEN(x) (ALTREP(x) ? 0 : STDVEC_TRUELENGTH(x))) with STDVEC_TRUELENGTH in forder.c and chmatch.c. You are only using it for character strings here which are not ALTREP. I have done this in {collapse}'s version of forder for like 3 years and never encountered an issue. More fundamentally, these algorithms wouldn't work anymore with ALTREP strings because all strings would yield 0. So if you like I'd make that replacement in forder.c and chmatch.c which should boost performance before you merge this (if you want to merge it that is).

[aitap]

Excellent idea, thank you! Hope it would be accepted by the CRAN team without causing, e.g., license problems due to MPL being GPL-compatible ("provided the combination is released under the same GNU GPL version") and the incorporated parts of Defn.h being GPL-licensed (unlike the installed headers, which are LGPL).

(There are other ways existing CRAN packages work around the non-API check, but this one seems more honest.)

[SebKrantz]

Maybe if you want to be very correct you need to adjust the license, see, e.g. the {collapse} LICENSE. But I'd not make a big deal about it. If we are doing it right, this should just pass through the autochecks without the need for human engagement. Only need to get rid of some NOTES still:

Found the following (possibly) invalid URLs:
URL: https://cran.r-project.org/web/checks/check_results_data.table.html
From: man/openmp-utils.Rd
README.md
Status: Error
Message: Connection died, tried 5 times before giving up
URL: https://twitter.com/hashtag/rdatatable
From: README.md
Status: 403
Message: Forbidden
URL: https://x.com/r_data_table
From: README.md
Status: 403
Message: Forbidden

Found the following (possibly) invalid file URI:
URI: Seal_of_Approval.md
From: README.md

[tdhock]

Should have only minor performance implications.

do you have any idea where those minor performance implications should be? It is difficult for me to tell, because there are so many files changed. It would be great to add a performance test.

@Rdatatable/performance-testers do you know why the atime CI job failed here?

[SebKrantz]

@tdhock the only performance implications come from coercions (VECSEXP) and (SEXPREC_partial *). But these are optimized by the compiler, so I think overall it is negligible.