search

RGF-team / rgf

Home repository for the Regularized Greedy Forest (RGF) library. It includes original implementation from the paper and multithreaded one written in C++, along with various language-specific wrappers.
378 stars 58 forks source link

check for R NOTEs in logs #337

Closed StrikerRUS closed 3 years ago

StrikerRUS commented 3 years ago

Refer to https://cran.r-project.org/doc/manuals/r-devel/R-ints.html#Tools and https://github.com/microsoft/LightGBM/blob/50e061f396d7c1b704e79371dea0ddf07680ba75/.ci/test_r_package.sh#L14-L16.

StrikerRUS commented 3 years ago

@jameslamb Sorry for disturbing you! Could you please take a quick look at this 1-line PR and point me to what I'm doing wrong?

export _R_CHECK_CRAN_INCOMING_REMOTE_=0
* checking CRAN incoming feasibility ... NOTE
Maintainer: ‘Lampros Mouselimis <mouselimislampros@gmail.com>’

The Date field is over a month old.
jameslamb commented 3 years ago

@jameslamb Sorry for disturbing you! Could you please take a quick look at this 1-line PR and point me to what I'm doing wrong?

no problem, always happy to help! Maybe you have to also set _R_CHECK_CRAN_INCOMING_=0 (based on https://cran.r-project.org/doc/manuals/r-devel/R-ints.html#Tools)?

I think that in LightGBM, we don't face this issue for two reasons:

  1. The package is on CRAN, so the "Maintainer:" part gets converted to "Note_to_CRAN_maintainers"
  2. We template the date field so it's always current at runtime: https://github.com/microsoft/LightGBM/blob/fd094d2719a04b984b376a665b6c3e5e1aff2967/build-cran-package.sh#L20, https://github.com/microsoft/LightGBM/blob/fd094d2719a04b984b376a665b6c3e5e1aff2967/build-cran-package.sh#L83

On this PR, I'd try setting _R_CHECK_CRAN_INCOMING_=0 and templating the date in DESCRIPTION the way we do with LightGBM.

StrikerRUS commented 3 years ago

@jameslamb Thanks a lot!

I don't think that (1) is a root case because RGF has been on CRAN for a long time: https://cran.r-project.org/package=RGF.

Right after a fresh CRAN upload The Date field is over a month old. transforms into NOTE : Days since last update: 1., which cannot be workaround by templating the date. So, I don't think (2) is a reason why we don't face this issue in LightGBM. It is something different, I guess.

jameslamb commented 3 years ago

The most recent build of this PR's CI when I made my comment showed this NOTE

* checking CRAN incoming feasibility ... NOTE
Maintainer: ‘Lampros Mouselimis <mouselimislampros@gmail.com>’

The Date field is over a month old.

That isn't about the time since CRAN upload, since it has been 2 years since an RGF upload to CRAN (https://cran.r-project.org/web/packages/RGF/index.html). That one is about comparing the current system clock's date to https://github.com/RGF-team/rgf/blob/26482658ffea64006a5e206a7ab3b9fb34131d67/R-package/DESCRIPTION#L5

This particular "incoming feasibility" section is interesting because it groups together multiple log messages under it, but the eventual status (NOTE vs. Note_to_CRAN_Maintainers) changes based on which of those messages you see. I expect that _R_CHECK_CRAN_INCOMING_REMOTE_=0 is already suppressing the "shortly after a CRAN upload" problem you mentioned, and that templating the date (or manually updating it to today's date) or setting _R_CHECK_CRAN_INCOMING_=0 would suppress "The Date field is over a month old".

StrikerRUS commented 3 years ago

@jameslamb

Seems that _R_CHECK_CRAN_INCOMING_ helped! Thank you very much for the hint!