search

d- / MTS

Multivariate Time Series Package for R
4 stars 5 forks source link

Rewrote the residuals calculation for the VARMA likelihood in C++. #5

Closed jonlachmann closed 2 years ago

jonlachmann commented 2 years ago

A cleaned up pull request! =)

d- commented 2 years ago

Thanks again for your contribution here. Could you fix this new NOTE from R CMD check --as-cran MTS?

* checking R code for possible problems ... NOTE
LLKvarma: no visible binding for global variable ‘da’
Undefined global functions or variables:
  da
jonlachmann commented 2 years ago

Hi! Thanks for the feedback! I have never submitted a package to CRAN, so this is useful to learn. I removed the references to the global variables, and also adjusted RcppExports.cpp to avoid any errors when running R CMD check --as-cran MTS.

Plesae let me know if you find any further problems.

d- commented 2 years ago

Amazing. Yeah - reviewers on CRAN usually reject submissions with any NOTE.

I'm still seeing: 

* checking line endings in C/C++/Fortran sources/headers ... NOTE
Found the following sources/headers not terminated with a newline:
  src/RcppExports.cpp

and

* checking compiled code ... NOTE
File ‘MTS/libs/MTS.so’:
  Found no calls to: ‘R_registerRoutines’, ‘R_useDynamicSymbols’

It is good practice to register native routines and to disable symbol
search.

See ‘Writing portable packages’ in the ‘Writing R Extensions’ manual.

The last NOTE doesn't seem to have been introduced by this PR though.

jonlachmann commented 2 years ago

I have now added a newline to the file mentioned. As before, let me know if there are any further issues.