[REVIEW]: The ppmData R-package for setting up spatial point process models

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@skiptoniam@elbeejay<!--end-editor-- Reviewers: @OwenWard, @mhesselbarth Archive: 10.5281/zenodo.7679406

Status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/b5288481bbd5f97602a71ce7b66273ef"><img src="https://joss.theoj.org/papers/b5288481bbd5f97602a71ce7b66273ef/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/b5288481bbd5f97602a71ce7b66273ef/status.svg)](https://joss.theoj.org/papers/b5288481bbd5f97602a71ce7b66273ef)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@OwenWard & @mhesselbarth, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @elbeejay know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

📝 Checklist for @mhesselbarth

📝 Checklist for @OwenWard

mhesselbarth commented 1 year ago

I have set some time aside this week!

@skiptoniam Which branch would be the best to look at the code?

mhesselbarth commented 1 year ago

I'm extremely sorry for this, but probably need one more week. I had a medical emergency this week which messed up my schedule this week and I wasn't really able to work as planned. Sorry for the inconveniences this is causing.

elbeejay commented 1 year ago

No problem @mhesselbarth, thank you for the heads up. Hope everything works out okay for you personally, this is definitely lower priority than your health!

OwenWard commented 1 year ago

I've had another look through the package and everything looks good. I marked two small issues but they're easy to fix. My only other comments are below.

I'm still a little confused about the need for the package.R file?
For the comparison in the README, I'm likely confused but is there any reason you don't use the snails data in the comparison with spatstat? Basically, why not make the comparisons as exact as possible?

Once those and the 2 issues are addressed I'm happy @elbeejay

elbeejay commented 1 year ago

@skiptoniam any updates on the two issues @OwenWard has flagged above?

@mhesselbarth certainly don't want to pressure you to finish your second pass here, but if you do have an updated timeline on when that might be possible please let us know.

Thanks all.

mhesselbarth commented 1 year ago

I am back at my laptop/desk today and planned to look at it Thurs/Friday! Sorry again.

mhesselbarth commented 1 year ago

Okay, again sorry for the delay! I finally had the chance to look at the package again. I pretty much agree with @OwenWard.

There still are two warnings thrown due to class(x) checks (instead of inherits()).
https://github.com/skiptoniam/ppmData/issues/32
I think the identical name of the package and the main function is still confusing to me. Interstingly, this also combines the documention from package.R and ppmData.R when using ?ppmData. I don't think this is an actual problem, but it definitely feels a bit strange and I'm not sure if this could introduce issues down to road. So, I would suggest to change probably the name of the main function e.g., fit_ppmData() https://github.com/skiptoniam/ppmData/issues/22

So once the first point is fixed, good to go from my side. However, again, I would strongly recommend to consider point two as well.

elbeejay commented 1 year ago

Thanks @mhesselbarth for taking a second look and noting your comments here.

@skiptoniam please comment here with any estimation of when you think you'll be able to handle these last few items pointed out by both @OwenWard and @mhesselbarth - they are minor comments but should be addressed.

skiptoniam commented 1 year ago

@elbeejay I'll be working on it today and tomorrow, so hopefully by the end of the week at the latest.

skiptoniam commented 1 year ago

Hi All,

I have update the comparison in the README as per @OwenWard first comment. I also think that this is a good idea and have updated the comparison to be run on the snails data for both bits of software.

I have removed the package.R file and moved any additional information (in the package.R file) into the ppmData.R file. I think this will deal with any conflict and issues associated with the package file. And should address point 1 from @OwenWard and point 2 from @mhesselbarth.

Apologies on missing that class(x) call as mentioned by @mhesselbarth, I thought I had caught all of those previously. I just ran the checks as part of the package build and they all appear to be dealt with now.

Thanks again to @elbeejay, @OwenWard & @mhesselbarth for all your really helpful suggestions and comments.

mhesselbarth commented 1 year ago

Good job! Congratulations on a cool package.

elbeejay commented 1 year ago

@skiptoniam a few minor things I'd like to close out before we move on to getting metadata set and everything ahead of publication.

As best I can tell, issue 30: https://github.com/skiptoniam/ppmData/issues/30 remains unresolved
It'd also be preferable (from the JOSS perspective) if you could, at the minimum, address, if not resolve, the other two outstanding issues in the repository that were raised as part of this review process

Thanks

skiptoniam commented 1 year ago

@elbeejay I have fixed those issues and closed them.

Cheers,

Skip.

elbeejay commented 1 year ago

Great, thanks @skiptoniam. At this time we need to finalize the metadata and versioning for the version of ppmData that will be tied to this JOSS publication. I noticed that the work related to the JOSS publication has happened in the "joss_submission" branch, this is also an opportunity to merge it into the "main" branch if you'd like, as the review process is over. Please let me know once you've been able to do the following:

[x] Make a tagged release of your software, and list the version tag of the archived version here.
[x] Archive the reviewed software in Zenodo or a similar service (e.g., figshare, an institutional repository)
[x] Check the archival deposit (e.g., in Zenodo) has the correct metadata. This includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it). You may also add the authors' ORCID.
[x] Please list the DOI of the archived version here.
[x] (if necessary) Re-release the package on CRAN so that the "JOSS version" matches one in a package repository

Thanks again to @OwenWard and @mhesselbarth for their work reviewing this package.

skiptoniam commented 1 year ago

@elbeejay Thanks.

I have made a release: https://github.com/skiptoniam/ppmData/releases/tag/v1.0.0 Archived the software here: https://doi.org/10.5281/zenodo.7679406 I am yet to put the software on CRAN, but will do so soon.

elbeejay commented 1 year ago

@editorialbot check references

elbeejay commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1890/10-1251.1 is OK
- 10.1111/1467-842X.00128 is OK
- 10.1016/j.ecolmodel.2005.03.026 is OK
- 10.1890/07-2153.1 is OK
- 10.1111/1365-2656.12071 is OK
- 10.1111/2041-210X.12352 is OK
- 10.1214/13-aoas667 is OK
- 10.1111/2041-210X.12242 is OK
- 10.1111/j.2041-210X.2011.00141.x is OK
- 10.1371/journal.pone.0079168 is OK
- 10.1214/10-AOAS331 is OK
- 10.1111/j.1467-9876.2011.01023.x is OK
- 10.2307/2347614 is OK
- 10.1111/anzs.12337 is OK
- 10.1111/j.1467-9876.2009.00701.x is OK
- 10.1111/j.1541-0420.2012.01824.x is OK
- 10.1093/biomet/asv064 is OK
- 10.1002/env.2194 is OK
- 10.1111/biom.12059 is OK
- 10.1016/j.spasta.2019.100392 is OK
- 10.1111/2041-210x.12782 is OK
- 10.1007/bf01386213 is OK
- 10.32614/RJ-2018-009 is OK
- 10.18637/jss.v033.i01 is OK
- 10.1007/bf02985802 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

elbeejay commented 1 year ago

@editorialbot set 10.5281/zenodo.7679406 as archive

editorialbot commented 1 year ago

Done! Archive is now 10.5281/zenodo.7679406

elbeejay commented 1 year ago

@skiptoniam thank you for doing that. A release on CRAN is not a requirement for JOSS, just a suggestion and a reminder I like for include as some R packages submitted to JOSS are already hosted on JOSS (and could be updated to match the JOSS version of record).

At this time I will be recommending ppmData for publication in JOSS. Sometime over the next week or so an editor-in-chief will review the paper and this issue to do a final check before officially publishing your article in JOSS.

elbeejay commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago

Attempting dry run of processing paper acceptance...

editorialbot commented 1 year ago

The paper's PDF and metadata files generation produced some warnings that could prevent the final paper from being published. Please fix them before the end of the review process.

a_i) - \lambda_i\} \numberthis 
                   ^
unexpected "\\"
expecting "&", "\\\\", white space or "\\end"

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1890/10-1251.1 is OK
- 10.1111/1467-842X.00128 is OK
- 10.1016/j.ecolmodel.2005.03.026 is OK
- 10.1890/07-2153.1 is OK
- 10.1111/1365-2656.12071 is OK
- 10.1111/2041-210X.12352 is OK
- 10.1214/13-aoas667 is OK
- 10.1111/2041-210X.12242 is OK
- 10.1111/j.2041-210X.2011.00141.x is OK
- 10.1371/journal.pone.0079168 is OK
- 10.1214/10-AOAS331 is OK
- 10.1111/j.1467-9876.2011.01023.x is OK
- 10.2307/2347614 is OK
- 10.1111/anzs.12337 is OK
- 10.1111/j.1467-9876.2009.00701.x is OK
- 10.1111/j.1541-0420.2012.01824.x is OK
- 10.1093/biomet/asv064 is OK
- 10.1002/env.2194 is OK
- 10.1111/biom.12059 is OK
- 10.1016/j.spasta.2019.100392 is OK
- 10.1111/2041-210x.12782 is OK
- 10.1007/bf01386213 is OK
- 10.32614/RJ-2018-009 is OK
- 10.18637/jss.v033.i01 is OK
- 10.1007/bf02985802 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 1 year ago

:wave: @openjournals/ese-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4000, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

kthyng commented 1 year ago

archive looks good ✅ I might have missed this, but is the version up to date?

kthyng commented 1 year ago

paper looks good ✅

Once we verify the version, this is good to go!

skiptoniam commented 1 year ago

@elbeejay thanks for all your hard work on this. @kthyng yes this is the most recent version. If you need me to reload the files or check it, then I can.

kthyng commented 1 year ago

@editorialbot accept

editorialbot commented 1 year ago

Doing it live! Attempting automated processing of paper acceptance...

editorialbot commented 1 year ago

The paper's PDF and metadata files generation produced some warnings that could prevent the final paper from being published. Please fix them before the end of the review process.

a_i) - \lambda_i\} \numberthis 
                   ^
unexpected "\\"
expecting "&", "\\\\", white space or "\\end"

editorialbot commented 1 year ago

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

editorialbot commented 1 year ago

🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘

editorialbot commented 1 year ago

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/4010
Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.04771
If everything looks good, then close this review issue.
Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

kthyng commented 1 year ago

I see the complaint about the latex generation, but the final paper looks good!

Congrats on your new publication @skiptoniam! Many thanks to editor @elbeejay and reviewers @OwenWard and @mhesselbarth for your time, hard work, and expertise!!

editorialbot commented 1 year ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.04771/status.svg)](https://doi.org/10.21105/joss.04771)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.04771">
  <img src="https://joss.theoj.org/papers/10.21105/joss.04771/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.04771/status.svg
   :target: https://doi.org/10.21105/joss.04771

This is how it will look in your documentation:

We need your help!

The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: https://reviewers.joss.theoj.org/join
Making a small donation to support our running costs here: https://numfocus.org/donate-to-joss

openjournals / joss-reviews