statnet / ndtv

ndtv: Network Dynamic Temporal Visualizations in R
https://cran.r-project.org/web/packages/ndtv/index.html
49 stars 5 forks source link

message from CRAN about ndtv removal from check Warnings #44

Closed skyebend closed 3 years ago

skyebend commented 3 years ago

Dear maintainer,

Please see the problems shown on https://cran.r-project.org/web/checks/check_results_ndtv.html.

Please correct before 2021-06-29 to safely retain your package on CRAN.

Packages in Suggests should be used conditionally: see 'Writing R Extensions'. This needs to be corrected even if the missing package(s) become available.

The CRAN Team

    Warning in data("florentine") : data set ‘florentine’ not found
skyebend commented 3 years ago

@krivit have the "florentine" example networks in tergm moved elsewhere?

krivit commented 3 years ago

tergm is currently broken. We are going to push an update to CRAN tonight or tomorrow.

skyebend commented 3 years ago
Package ‘ndtv’ was removed from the CRAN repository.
Formerly available versions can be obtained from the archive.
Archived on 2021-07-13 as problems were not corrected in time.
A summary of the most recent check results can be obtained from the check results archive.
Please use the canonical form https://CRAN.R-project.org/package=ndtv to link to this page.

I believe I had fixed the problems with docs (in master branch) and tried to push a 0.13.1 update on 7-11. Although it passed local, Rhub, and winbuilder checks, it was still rejected by CRAN and I ran out of time that weekend :cry: No email notification (just noticed when I tried to work on it again tonight)

krivit commented 3 years ago

Huh... Actually, florentine is in ergm, not tergm; not sure what happened here.

martinamorris commented 3 years ago

Just a note that a fix is urgently needed. The NME workshop starts 8/2 -- and we rely on ndtv.

skyebend commented 3 years ago

The original problem was just that the ergm and tergm were in a strange state during the rollout. Once that resolved, there was a problem with a change to ergm's citation format breaking the ndtv vignette which I corrected, along with some NOTEs about urls in docs needed updates. I'll try to resubmit to CRAN again tomorrow ... but now it is a 'new' package and probably has a longer review process.

in the meantime, we may have to install from github instead of CRAN with devtools::install_github('https://github.com/statnet/ndtv',subdir='ndtv')

martinamorris commented 3 years ago

@skyebend for the NME workshop that would mean people would need to install devtools or remotes, which is not ideal.

workshop prep email is going out today, it's meant to include software installation instructions. so there's some urgency now.

is there anything i can do to help?

martinamorris commented 3 years ago

Note that most of our other packages are now automatically built from GH master and hosted at the runiverse site. Not sure why ndtv is not included, but we might want to find out, as this is a good alternative to CRAN in a crunch.

smjenness commented 3 years ago

Fortunately, ndtv doesn't require compilation, so the install with remotes should be no problem in case it doesn't make it to CRAN in time.

skyebend commented 3 years ago

Still not able to confirm any errors with the package. I've reattempted submission, fingers crossed!

skyebend commented 3 years ago

submission was rejected. This time one of the NOTEs is that overall testing of the package is now taking slightly too long on windows

Flavor: r-devel-windows-ix86+x86_64
Check: Overall checktime, Result: NOTE
  Overall checktime 11 min > 10 min

I think the longest running part of the checks are generating the term sims in the vignettes (about 5min on my laptop) https://github.com/statnet/ndtv/blob/26b22e9063c353fed85f1450079508348d321311/ndtv/vignettes/ndtv.Rnw#L110

@krivit I assume we must run much larger simulations in the tergm vignettes, how do we work around the time limits there? Or is there an alternative model that would still generate useful output but might run slightly faster?

I've also emailed r-devel for guidance about NOTEs about temp files that don't seem to be produced by ndtv

chad-klumb commented 3 years ago

It appears tergm was pushed to CRAN without its vignette, probably due to a combination of slowness and its having not been updated for 4.0.

There is a bug in CRAN tergm's EGMME that's been fixed in tergm master; running the following script with tergm master

require(tergm)
require(ndtv)
data("florentine")

theta.diss <- log(9)

tl <- list()

for(i in seq_len(10)) {
  set.seed(i)
  st <- Sys.time()  
  stergm.fit.1 <- stergm(flobusiness,
                         formation = ~edges + gwesp(0, fixed=TRUE), 
                         dissolution = ~offset(edges),
                         targets="formation",
                         offset.coef.diss = theta.diss,
                         estimate = "EGMME")
  et <- Sys.time()
  print(et - st)
  tl[[i]] <- et - st
}

gives me fit times ranging from 42 seconds to 3.3 minutes, with a mean of 1.42 minutes and an sd of 0.75 minutes. An occasional 5 minute (or longer) fit time is still possible, but should be less likely than with CRAN tergm.

The updates in tergm master will probably be pushed to CRAN very soon, but if you can't wait for that, you could push ndtv without the vignette, or without actually calling stergm() in the vignette (neither is ideal, of course, but they avoid the wait).

martinamorris commented 3 years ago

since ndtv will be treated as a new package, taking longer to approve, it might be worth submitting sans vingnette to get it re-established, and submitting again with vignette after the tergm fix is on CRAN

chad-klumb commented 3 years ago

re-ran with seeds 1 thru 100 (using master); mean fit time was 1.27 minutes, sd 0.95 minutes, and only 1 out of 100 was over 4 minutes (it was 8.52 minutes), so longer times are possible but appear fairly rare

skyebend commented 3 years ago

resubmitted ndtv without vignette (assuming we can upload another version in a month or so)

skyebend commented 3 years ago

rejected, even without vignette. Still getting

Flavor: r-devel-linux-x86_64-debian-gcc
Check: for detritus in the temp directory, Result: NOTE
  Found the following files/directories:
    'calibre_5.12.0_tmp_7sa77ntk' 'calibre_5.12.0_tmp_81_3ufjg'
    'calibre_5.12.0_tmp_8ighpiy8' 'calibre_5.12.0_tmp_9c28qvuu'
    'calibre_5.12.0_tmp_9cb7j2sq' 'calibre_5.12.0_tmp_a3dewyxk'
    'calibre_5.12.0_tmp_eda6atf2' 'calibre_5.12.0_tmp_g251ao33'
    'calibre_5.12.0_tmp_h21zpa9s' 'calibre_5.12.0_tmp_lwbe9tur'
    'calibre_5.12.0_tmp_oq2xt29x' 'calibre_5.12.0_tmp_xsmi81qn'

which I haven't been able to reproduce or find code reference for. I've emailed to beg for mercy

krivit commented 3 years ago

Calibre, the e-book manager?

Can you try opening up the tar.gz file and checking if the files are in there somehow?

martinamorris commented 3 years ago

@krivit did you get Uwe Ligges' email to @skyebend -- he identified the issue and a fix. i managed to delete that. would be good to add that here if possible.

krivit commented 3 years ago

I don't have any records of an e-mail from Uwe Ligges about ndtv.

martinamorris commented 3 years ago

not tsna, ndtv

krivit commented 3 years ago

I mistyped; I did mean ndtv.

skyebend commented 3 years ago
On 25.07.2021 07:17, Skye Bender-deMoll wrote:
> Dear CRAN,
>
> When attempting to resubmit the ndtv package, I'm still encountering the 
> following error on on the Debian precheck:
>
> * checking for detritus in the temp directory ... NOTE
> Found the following files/directories:
>    ‘calibre_5.12.0_tmp_7sa77ntk’ ‘calibre_5.12.0_tmp_81_3ufjg’
>    ‘calibre_5.12.0_tmp_8ighpiy8’ ‘calibre_5.12.0_tmp_9c28qvuu’
>    ‘calibre_5.12.0_tmp_9cb7j2sq’ ‘calibre_5.12.0_tmp_a3dewyxk’
>    ‘calibre_5.12.0_tmp_eda6atf2’ ‘calibre_5.12.0_tmp_g251ao33’
>    ‘calibre_5.12.0_tmp_h21zpa9s’ ‘calibre_5.12.0_tmp_lwbe9tur’
>    ‘calibre_5.12.0_tmp_oq2xt29x’ ‘calibre_5.12.0_tmp_xsmi81qn’

Looks like these are leftovers from a browser you are starting during 
the checks.

Note that this is not permnitted. Please only start web browsers in 
interactive mode, i.e. protect it via

if(interactive()){
  .....
}

Please fix and resubmit.

Best,
skyebend commented 3 years ago

Made the updates suggested by Uwe, and resubmitted. Also re-added vignette since it is now building in ~4min with updated tergm

skyebend commented 3 years ago

Accepted!

Thanks, on its way to CRAN. Best, Uwe Ligges

and looks like it was built cleanly for all platforms: https://cran.r-project.org/web/checks/check_results_ndtv.html