Cleanup README - add badges and fit init.h issue

[k-doering-NOAA]

I've come across a couple additional checks that are required before submitting to rOpenSci:

1. pkgcheck() from the pkgcheck library
2. autotest_package() from the autotest library

We may or may not want/need to submit FIMS to rOpenSci, but the checks are still useful in detecting potential problems or inefficiencies in a package.

Originally posted by @Andrea-Havron-NOAA in https://github.com/NOAA-FIMS/FIMS/issues/55#issuecomment-1095290354

[Bai-Li-NOAA]

I've checked autotest_package() from the autotest library.

• It runs the tests under the testthat folder
• It checks if a file needs a test by scraping documented examples from all .Rd files (not all R files).
• The results would be NULL if all tests pass. The output indicated that FIMS::mkObj() needs a test.

  
  -- autotesting FIMS --

v [1 / 1]: mkObj

Currently, we use devtools::test() for FIMS.
- devtools::test() runs the tests under the testthat folder. 
- devtools::test_coverage() computes test coverage for the package by scraping all files under R/ and src/. (Maybe we can add the coverage table to FIMS readme?)
- test_coverage() returns a summary table and an interactive report that shows which lines of the code need a test.

FIMS Coverage: 27.78% R/io.R: 0.00% R/zzz.R: 0.00% src/FIMS.cpp: 0.00% src/init.hpp: 100.00%

At this moment, devtools::test() and autotest_package() give very similar feedback. Could check the two functions again when we have more R code in the repo.

@Andrea-Havron-NOAA, I am not able to install pkgcheck through CRAN (R version issue) or GitHub (dependency issue). It would be great if you have time to take a look at the pkgcheck and compare the summary of check results between CRAN devtools::check() and pkgcheck. Thanks!

[Andrea-Havron-NOAA]

The pkgcheck() returns the following:

√ Package name is available
x does not have a 'codemeta.json' file.
√ has a 'contributing' file.
√ uses 'roxygen2'.
x 'DESCRIPTION' does not have a URL field.
x 'DESCRIPTION' does not have a BugReports field.
x Package has no HTML vignettes
√ All functions have examples.
x Continuous integration checks unavailable (no URL in 'DESCRIPTION').
x Package coverage is 27.8% (should be at least 75%).
√ R CMD check found no errors.
x R CMD check found 4 warnings.

So pkgcheck() runs devtools::check() and test_covergae() in additional to other checks. I know one useful function being run is cylocomp::cyclocomp() which checks the cyclometric complexity of the R scripts. Most of the items being flagged pertain to our DESCRIPTION file and setting up appropriate badges in the README. I think we would only want to add the codemeta.json file if we want to submit the package to rOpenSci (easily done with usethis).

With respect to test_coverage(), we will want to set up codecov on FIMS using codecov::covr(). This creates a page in gitHub with all the test coverage stats (see r4ss example here) and linked through the codecov badge in README. Both @k-doering-NOAA and I have set this up before.

The warnings and notes from devtools::check() are:

> checking if this is a source package ... WARNING
  Subdirectory 'src' contains:
    init.hpp
  These are unlikely file names for src files.

> checking for executable files ... WARNING
  Found the following executable files:
    a.exe
    tests/compilation_tests/a.exe
  Source packages should not contain undeclared executable files.
  See section 'Package structure' in the 'Writing R Extensions' manual.

> checking compilation flags in Makevars ... WARNING
  Non-portable flags in variable 'PKG_CXXFLAGS':
    -w -O3

> checking for hidden files and directories ... NOTE
  Found the following hidden files and directories:
    .vscode
  These were most likely included in error. See section 'Package
  structure' in the 'Writing R Extensions' manual.

> checking installed package size ... NOTE
    installed size is 19.6Mb
    sub-directories of 1Mb or more:
      libs  19.5Mb

> checking top-level files ... NOTE
  File
    LICENSE
  is not mentioned in the DESCRIPTION file.
  Non-standard files/directories found at top level:
    'a.exe' 'docs'

> checking compiled code ... NOTE
  File 'FIMS/libs/i386/FIMS.dll':
    Found '_ZSt4cout', possibly from 'std::cout' (C++)
      Object: 'FIMS.o'
  File 'FIMS/libs/x64/FIMS.dll':
    Found '_ZSt4cout', possibly from 'std::cout' (C++)
      Object: 'FIMS.o'

  Compiled code should not call entry points which might terminate R nor
  write to stdout/stderr instead of to the console, nor use Fortran I/O
  nor system RNGs.

It looks like we need to clean up FIMS a bit (remove a.exe, add .vscode to .gitignore). My R pacakge uses an init file in the src but doesn't throw a warning but I use the extension init.h instead of init.hpp. I suggest we change the extension to avoid the warning. Do we need the -w -O3 compiler flags? The compiler automatically adds -O2 as a flag.

I propose creating a branch to:

• [ ] Clean up the repository
• [x] Add URLs to the DESCRIPTION file
• [x] Set up codecov for R tests
• [x] Set up codecov for C++ tests
• [x] Add a coverage and r-cmd-check badge (also lifecycle?) to README
• [x] Add installation instructions to README
• [x] Change init.hpp to init.h
• [x] remove a.exe
• [x] add .vscode to .gitignore
• [x] remove -w -O3 flags
• [ ] update doxygen documentation (interface.hpp, FIMS.cpp, def.hpp, and files under rcpp/rcpp_objects)

Rather than requiring developers run pkgcheck(), I suggest in addition to devtools::check(), we also ask developers to run devtools::test_coverage() and cyclocomp::cyclocomp() if changes are being made to R scripts.

[k-doering-NOAA]

These are all great ideas!THanks @Andrea-Havron-NOAA and @Bai-Li-NOAA for looking into this.

I've never set up code coverage for an R package that has C++ code; I'm curious if there is a way for us to set it up so we get "credit" for the coverage of the google tests? Or maybe this already an option that I'm just not aware of.

[ChristineStawitz-NOAA]

I think the new README looks great @Andrea-Havron-NOAA ! Thank you!

[Andrea-Havron-NOAA]

@Bai-Li-NOAA and @ChristineStawitz-NOAA,

I commented out the std::cout code in the Interface.hpp file and re-ran R CMD check. This removed the note:

> checking compiled code ... NOTE
  File 'FIMS/libs/i386/FIMS.dll':    
  Found '_ZSt4cout', possibly from 'std::cout' (C++)
      Object: 'FIMS.o'
  File 'FIMS/libs/x64/FIMS.dll':
    Found '_ZSt4cout', possibly from 'std::cout' (C++)
      Object: 'FIMS.o'

So it does look like it is our code throwing this issue and is not related to the recent TMB release.

From my understanding, this is just placeholder code and the final version of FIMS will not use any std::cout statements. If that is correct, we can leave the std::cout code as is and ignore this note for now.

[Bai-Li-NOAA]

@Andrea-Havron-NOAA , I also think the std::cout code in the Interface.hpp file is just placeholder code and we can ignore this note for now.

I reopened the issue because we have some unfinished tasks. I am working on linking codecov with C++ tests (done).

We may need help from @msupernaw to add more doxygen documentation for a few files.

I could not find documentation for fims_math.hpp. According to doxygen manual, it could be because "the functions are overlooked: to document global objects (functions, typedefs, enum, macros, etc), you must document the file in which they are defined. "

[Bai-Li-NOAA]

When generating doxygen htmls locally, we got more warnings. Please see all the warnings under Building HTML documentation with Doxygen section. @msupernaw, could you add documentation in interface.hpp, FIMS.cpp, def.hpp, and files under rcpp/rcpp_objects? Thanks!