search

cbreto / panelPomp

R package for statistical inference using panelPOMPs (panel Partially Observed Markov Processes)
11 stars 6 forks source link

small edits to tutorial #58

Closed kingaa closed 2 months ago

kingaa commented 2 months ago

This is just a few edits to the tutorial.

kingaa commented 2 months ago

Have you given thought to the version-numbering scheme and the release schedule? The version numbering I use in pomp (X.Y.Z.W). The X.Y match the latest CRAN release, while the Z.W reflect changes that are user-visible (Z) or invisible (W).

Also, it seems that we haven't been making regular releases. This is useful, I think. At the very least, the current release should be at least as new as the CRAN release. That is not the case at present.

kingaa commented 2 months ago

The errors in the R-devel checks seem to be the same ones that are showing up in the CRAN checks: some issues with \link{} markup in the help pages.

jeswheel commented 2 months ago

Thanks for the edits. Just about everything looks good, but there is an extra R CMD check note generated due to the existence of the params.json file at the top level of the project. Previously, this was ignored in the .Rbuildignore file, but your changes no longer ignore this file and we get the note. Before I merge this PR, this file should be added back to the .Rbuildignore or should be deleted from the repository.

I'm not exactly sure what the intended purpose of this file is. I think that it can be deleted, but Carlos made a specific comment in the file: "Don't delete this file! It's used internally to help with page regeneration.", so I didn't want to delete it without checking with someone else. If any of the other core developers agree that it is not needed, then I am happy to delete the file.

kingaa commented 2 months ago

Sorry I missed that. There are a few other files I inadvertently removed from the .Rbuildignore file. I'll amend the pull request. I'll also try to figure out what's going on with the bad \link{} markup.

kingaa commented 2 months ago

I think the pull request now resolves the issue with the .Rbuildignore as well as with the malformed cross-references in the help pages (flagged on CRAN).

By the way, can you give me push permission on this repo so that I don't have to make pull requests from a fork?

jeswheel commented 2 months ago

The fixes look good from my end.

While I have permissions to merge pull-requests, I do not have the ability to give you push permissions directly to this repo as I'm not the owner of the repository (I also don't have direct pushing abilities either). @cbreto, is this something that you could give to the core developers for this package (Aaron, Ed, and myself)?

kingaa commented 2 months ago

@jeswheel : you'll need to rebuild the tutorial. I only pushed the .Rnw file.

kingaa commented 2 months ago

I think the javascripts directory and the params.json file are left over from the old website, no? If so, then they can be safely deleted, correct?

cbreto commented 2 months ago

I would tend to agree. I'm all for removing params.json and see if it's still being used at all to regenerate the panelPomp github page.

jeswheel commented 2 months ago

@jeswheel : you'll need to rebuild the tutorial. I only pushed the .Rnw file.

Good catch, I just rebuilt and pushed to the repository.

I think the javascripts directory and the params.json file are left over from the old website, no? If so, then they can be safely deleted, correct?

I think you're correct, and Carles seems to be happy with removing them as well, so I will remove these files shortly.