Closed ShanaScogin closed 3 years ago
Added conditional in 99c6e7b57e00a8c4005f9741bd210fae63f74f0c. Winbuilder doesn't have any warnings, though now I'm getting a NOTE with some other dependency issue with MCMCpack. Will update on that.
Package unavailable to check Rd xrefs: 'MCMCpack'
(Also thanks to this issue in bayestestR for the conditional code! https://github.com/easystats/bayestestR/issues/276)
~Never mind - warning is back up.~
~edit: I'm wondering if the warning (same as in #78) is a makefile issue specific to rstan. Not sure exactly what could be going on in BayesPostEst that would do that, but I'll keep looking.~
I'm striking this out bc I'm pretty sure it's not related to this.
Looks like ggridges, which is used in mcmcFD()
is the last one on this list that we don't use conditionally. I'll check out their github to see what they're doing since I'm not sure how we'd remove it from mcmcFD()
ggplot is on the list, too - really confused by this.
Woo! Looks like this should be solved - it was a small test hiccup that everyone is sorting out, so I'm going to push this release with the minor dependency and conditional changes, and hopefully that's it for now! (Latest commit in this whole ordeal is 430cf33c7444e6c2b81206fb9278a8b9174771ff - and yes, I need to learn to squash and stop with the typos....)
Going to reopen this and fold in the to do list from the release that is trying to push this
@andybega @jayrobwilliams @jkarreth - Thoughts on size?
Over half of the memory footprint appears to be test data:
Not sure what the best solution, but if the test data is necessary, perhaps it could be compressed in the repo and then decompressed at test runtime?
Looks like there are two data & data-raw: 1.) one in test and 2.) one in the main folder. I need to see what the data in the main folder is for, but if it's for examples, maybe we can pre-run and then remove that?
I'm seeing these data are used for tests and examples. I'm looking more into this and I'm guessing we decided to include the data so that there wasn't so much simulation code (maybe also for run time). Perhaps we can 1.) include a separate vignette with all the examples and the simulation code that the main function documentation can point to, 2.) delete the data for the examples, 3.) only have one type model simulated for the function documentation example. I can look into how to have the tests run without having the actual datasets in the build. Thoughts are welcome - I do need to do this by the 12th per CRAN (for the other stuff), so I might do a fix that works now and then we can enhance it later.
Edit: Also I forgot to thank you Andrew! @ZenPylon :D
I don't think test
and data-raw
are included in the built package, i.e. they don't count against that 5MB note.
@andybega - Thanks! I was hoping that, too, but they do seem to, unfortunately. I don't know if it's new, or if there's something else I need to do to make sure of that? I'll look more into it soon. Anyway, I didn't touch those for now - I just messed with the ones in the main folder. So:
1.) I removed the .rda files for a.) two bugs models, b.) the runjags interactive model, and c.) the mcmcpack linear model in the main data folder. NOTE: mcmcpack was a terrible choice to delete (but it was large...) bc the randomness means I have to set high tolerances for one run and I had to comment out a chunk in mcmcReg()
test to deal with later.
2.) I put the simulation code conditionally in the tests to get us down to 4,984,146 bytes (5 MB on disk). (Cutting it close........)
3.) I kept the raw data files in case they want to be made again.
This can all be seen in commit 3f68eedca9cd770a682a1789a803e9a484a60986.
Checks (cmd and gh-actions) seem to passing right now (about to do winbuilder), so, I might try to resubmit to CRAN to see what else they have to say and make a non-priority issue to clean up these data files/make another vignette on simulating the data (just for educational purposes in case Johannes et al use this in classes), etc.
I was just looking through .Rbuildignore
and I was wrong, tests
is included in the package.
Hey, btw, did you manually rename master
to main
or did GitHub do that? Trying to figure out how to update my fork ATM.
Oof - too bad - I'll def make it a to-do to figure out something since having the static objects really does help for some of them. We might could make a supportive package that just lives on my github maybe? I'll see.
And ya! Sorry about that - there is a section in here that worked for @jayrobwilliams, I'm pretty sure: https://www.git-tower.com/learn/git/faq/git-rename-master-to-main/
Thanks. I think I got it working again with main
now.
Just updated main
with your latest changes and I get a clean R check now as well (no NOTE about 5MB).
FWIW, looking at the built package tarball, seems that the BRMS and RStan logit examples in tests/testdata
are the big size offenders. Maybe it's possible to cut some elements from those objects that are not really needed in the tests. (Like how you can reduce the size of a fitted GLM model by cutting out the training data copy, etc.)
(tests/testdata
itself is 4.1 MB)
Thank you for correcting this @ShanaScogin and sorry to be late to this.
make another vignette on simulating the data (just for educational purposes in case Johannes et al use this in classes)
I can put that on my agenda.
Hi @jkarreth! No worries! I know it's the holidays - I just wanted to get this done so we don't get kicked off again! I'll start a file after I finally get this release accepted with the simulations you and Andy already made, and perhaps you can build on it if it's not enough? I'm doing another project where I need to simulate some stuff and I have loved to come across how-tos, so I think it'd be a neat addition to the package website.
Update on release: winbuilder found a couple urls that it doesn't like still, so I'm doing that now. Am fixing and rerunning now. If yall start working in develop I'm going to stay in Main till I can get this release accepted and then merge backwards.
Update2: current error (no more info - that's the end of the line)
** running tests for arch 'i386' ... ERROR
Check process probably crashed or hung up for 20 minutes ... killed
Most likely this happened in the example checks (?),
I'm going to put some code around some of the tests I just did to not run on CRAN since I'm assuming that's where the issue is. Good news is I seem to have finally caught all the urls that want to be changed (other than it never seems to like the ICPSR one - I'm just going to ignore that.)
Ok! Second attempt at a release for this week has been sent! I did have to comment out a couple of the tests I had just written that simulate in real time - so that might drive home how challenging simulating some of this data for the tests can be. If this release goes through, I'll start helping with the effort to making the objects smaller/finding another solution!
Gah. I thought I caught everything (check_rhub() finds more than winbuilder, which finds more than cmd check) but they found one more. They don't mind the https://doi.org/10.2139/ssrn.2765419 format in any other documentation but seem to mind it in the mcmcRocPrc
documentation. It wants doi/10.2139/ssrn.2765419 I guess?
After I change this, I will resubmit it (fourth time's a charm...)
Thanks, we see:
Found the following URLs which should use \doi (with the DOI name only): File 'mcmcRocPrc.Rd': https://doi.org/10.2139/ssrn.2765419
Please fix and resubmit.
Edit1: Maybe it wants doi:... ? https://owl.purdue.edu/owl/research_and_citation/conducting_research/internet_references/urls_vs_dois.html
Edit2: does it want this? \doi:10.2139/ssrn.2765419
Edit3: ok I'm adding this - I really hope this works bc the checks aren't finding it and if they deny this again we might fall off CRAN again.
#' @references Beger, Andreas. 2016. “Precision-Recall Curves.” Available at
#' \doi{10.2139/ssrn.2765419}
I think you're right - that (using \doi{ ...}
) is the only solution I can find as well!
Similar issues would then appear in
Do you want to change these as well? If it helps, please let me know and I can edit those soon (tonight)
Second thought - you say that they didn't mention any other instances of doi, so probably no need to change the above four files then!
Okay! Deleted last comment bc it was useless, but latest release is out the door! Note to future self- make sure you're using winbuilder develop, not release (every time!). (My bad.)
I'll get working on the doi stuff in the other documentation and start on the educative/example simulation vignette. I also look forward to seeing what we can do with the size/objs/tests.
Happy new year everyone!
Per email from CRAN: