openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
721 stars 38 forks source link

[REVIEW]: shar: A R package to analyze species-habitat associations using point pattern analysis #3811

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: @mhesselbarth (Maximilian H.K. Hesselbarth) Repository: https://github.com/r-spatialecology/shar Version: v1.3 Editor: @Bisaloo Reviewer: @lionel68, @tretherington Archive: 10.5281/zenodo.5761583

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

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

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

@lionel68 & @tretherington, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @Bisaloo 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

Review checklist for @lionel68

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @tretherington

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @lionel68, @tretherington it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.15 s (767.5 files/s, 135791.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
HTML                            42           2423            552           8681
R                               42           1453           1409           2572
Markdown                         6            211              0            824
CSS                              3             99             48            428
JavaScript                       4             64             34            266
YAML                             7             39              2            265
Rmd                              5            105            188             64
C++                              2             12             34             31
SVG                              1              0              1             11
-------------------------------------------------------------------------------
SUM:                           112           4406           2268          13142
-------------------------------------------------------------------------------

Statistical information for the repository 'e1c6cbbc50b85c598084e2ce' was
gathered on 2021/10/11.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Maximilian Hesselbar             6           404            110           51.87
marcosci                         1           185              0           18.67
mhesselbarth                     2           145            147           29.47

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Maximilian Hesselbar        407          100.7         15.2               17.44
marcosci                     70           37.8          0.0                7.14
whedon commented 3 years ago

PDF failed to compile for issue #3811 with the following error:

 Can't find any papers to compile :-(
Bisaloo commented 3 years ago

@whedon generate pdf from branch joss

whedon commented 3 years ago
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
whedon commented 3 years ago

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

Bisaloo commented 3 years ago

:wave: :wave: :wave: @mhesselbarth @lionel68 @tretherington this is the review thread for the paper. All of our communications will happen here from now on.

Both reviewers have checklists at the top of this thread with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#3811 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 4-6 weeks. Please let me know if any of you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@bisaloo) if you have any questions/concerns.

lionel68 commented 3 years ago

Ok just made a quick first go through the manuscript and still have to check the functionalities of the package itself so right now I will focus on the text:

  1. The species-habitat association derived from point pattern as used in shar are based on null models, there are however other ways to estimate these associations by directly modelling the intensity of the point pattern to habitat / covariates, I am thinking of inlabru::lgcp or spatstat::kppm. These other, more direct, approaches to estimate species-habitat association should be mentioned in the text

  2. The text is in some part a bit confusing and could be improved:

    • Line 8: "Studying species-habitat associations is one tool to reveal ... ", I find it weird to refer to studies as tools, maybe replace tool with "approach" or similar?
    • Line 15-19: I was a bit confused in this part about the ecological level, I usually envision point pattern analysis as being done within a species with individuals clustering in certain habitats and environmental conditions. Analysis of species-habitat associations is usually done within a species (as is the case in this package), I found these sentences therefore confusing concerning which levels where under interest here.
    • Line 26-28: The definition of point pattern is rather terse, I would expand and add concrete examples to make it clearer to the readers which datasets would qualify as a point pattern and which not.
  3. It would be interesting to have some ideas on which raster / point pattern size would give results in a reasonable amount of time with the package. Like how long does it takes to run 10'000 simulations of a raster with 1Mio pixels? And are there options / functionalities built in the package to run these computations on large dataset efficiently (parallel computing, GPU ...)

Voila that's it for now, I'll look at the package R functionalities in the next days.

tretherington commented 3 years ago

Kia ora (a New Zealand hello) @mhesselbarth, @lionel68, and @Bisaloo!

This is definitely an interesting package, and I've been keen to explore the use of null models more in the ecological niche modelling work that I have been doing of recent, so it was interesting to see other examples of studies randomising locations or landscapes to assess the significance of patterns. :smile:

I have had a first look through the paper and package, and have cross-referenced against the JOSS list of review criteria. To try and be clear I have opened a series of issues, one for each 'thing' so that there is a to do list in which things can be discussed:

That is a longish initial list, but most are very simple fixes, and please do ask me to clarify anything that I have not explained/described well enough. :smile:

mhesselbarth commented 3 years ago

Thanks for the helpful and good comments. I will start working on them and push changes to the joss branch!

@lionel68

@tretherington

mhesselbarth commented 3 years ago

@lionel68

whedon commented 3 years ago

:wave: @tretherington, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @lionel68, please update us on how your review is going (this is an automated reminder).

mhesselbarth commented 3 years ago

I am on holidays this week, but will address all remaining open issues next week!

Bisaloo commented 3 years ago

Thanks for letting us know! I'll be out of office next week myself. Enjoy your holidays!

/ooo October 30 until November 7

mhesselbarth commented 2 years ago

Okay, I think from my side I tried to address all comments by @tretherington and @lionel68.

tretherington commented 2 years ago

Sorry, I do have other thoughts, but have been swamped by a few tasks I've had to prioritise, but I'm trying to reserve some time tomorrow afternoon to provide more feedback to keep things moving along.

mhesselbarth commented 2 years ago

Issues that still need to be addressed:

mhesselbarth commented 2 years ago

@Bisaloo
I am planning to use v1.3 for the next release with all the helpful comments and suggestions by the reviewers. The first @whedon comment in this thread says v1.2.2. Not sure if thats important.

tretherington commented 2 years ago

I have just tried to run over all the points again, and for me I think the only outstanding one might be: "State of the field: Do the authors describe how this software compares to other commonly-used packages?"

At the moment the paper doesn't refer to any other packages that might be commonly used to do species-habitat analyses. I acknowledge that the package is quite possibly unique in that it does habitat analyses that account for the spatial patterns in the occurrence and habitat data, as opposed to more classical methods that just bin the data and assume randomness. But I do wonder if it would be helpful to make reference to some commonly used R packages that apply these more simplistic (perhaps incorrect!?) techniques as I think it could help a potential user to create a cognitive connection between how they currently do things, and how they could do things better by using shar.

For example, if I were to read that a function in a package I commonly use might be inappropriate then I think that is more likely to grab my attention as that provides a massive indicator of relevance. So perhaps at the end of the first paragraph of the statement of need section in the paper where you describe the sub-optimal approach you could give some examples of packages that include that sub-optimal functionality?

The problem I have here is that the species distribution/ecological niche modelling I tend to do is not really comparable to this approach - as I use continuous variables to produce a continuous model rather than a table of results from categorical data. So I'm unable to readily identify R packages that do similar habitat analyses. From vague memory I think the adehabitatHS package might be a good example as it includes a compana function for compositional analysis of habitat use that I think would qualify as an example of an approach that assumes statistical independence of the observations. But after that I am stuck, but perhaps you will have a better idea given you have more experience in this area.

Apologies that this isn't a very specific comment 😬 but hopefully that makes some sense?

Bisaloo commented 2 years ago

I am planning to use v1.3 for the next release with all the helpful comments and suggestions by the reviewers. The first @whedon comment in this thread says v1.2.2. Not sure if thats important.

That's not a problem. Feel free to increment the version number. We ask you to do it right before acceptance anyways. I'll tell whedon what version should be marked as accepted later.

mhesselbarth commented 2 years ago

@tretherington https://github.com/r-spatialecology/shar/commit/3850fb113935544b61fa033e637cba45e0ebb365 (https://github.com/r-spatialecology/shar/commit/2c77b3dd63de174161357c79a0e522ebc2936051)

I changed the last 1,2 paragraphs of the statement of need section quite a bit trying to compare shar more to other existing packages and methods.

However, I don't think that including adehabitatHS makes too much sense, since it is not really based on a spatially explicit point pattern approach, but rather on matrices of habitat use.

tretherington commented 2 years ago

Thanks for this. I understand that there may not be anything comparable in terms of methods to shar, but I guess I was thinking that if everyone is using a package that uses the equivalent suboptimal methods, then pointing out that limitation would be helpful. But you are the subject expert here, so I'm happy to go with what you have done. 😄

tretherington commented 2 years ago

@Bisaloo I've now ticked off everything in my reviewer checklist 🥳 but as a first time reviewer for JOSS if I've missed a step please let me know.

Bisaloo commented 2 years ago

@Bisaloo I've now ticked off everything in my reviewer checklist partying_face but as a first time reviewer for JOSS if I've missed a step please let me know.

@tretherington, you're all good. Thank you so much for your very helpful comments! :100:

Bisaloo commented 2 years ago

@lionel68, could you please go over you checklist in this comment? After reviewing each item, either check it or open an issue in https://github.com/r-spatialecology/shar/issues if you think it should be improved. Thanks :pray:

mhesselbarth commented 2 years ago

@lionel68 Also, feel free to re-open any issues I already closed! Here is a list with all your already closed issue: https://github.com/openjournals/joss-reviews/issues/3811#issuecomment-948515516

lionel68 commented 2 years ago

Sorry for the long wait, will look at this this afternoon, @mhesselbarth is there a new article proof, would be easier for me than going through the md file.

Bisaloo commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

PDF failed to compile for issue #3811 with the following error:

 Can't find any papers to compile :-(
Bisaloo commented 2 years ago

@whedon generate pdf from branch joss

whedon commented 2 years ago
Attempting PDF compilation from custom branch joss. Reticulating splines etc...
whedon commented 2 years ago

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

lionel68 commented 2 years ago

Thanks @whedon

Some further / last minor comments:

  1. line 24: I would rephrase, "Spatial autocorrelation of the environmental conditions could also ..."
  2. line 27: this sentence is a bit cryptic regarding the method developed in the package, maybe best to directly name it? Like "Previous research have shown that violation of independence assumptions led to higher number of false-positive habitat associations when using method XX compared to method YY"
  3. line 104: didn't function reconstruct_pattern changed in previous commit beec06e ?

Otherwise looks good!

mhesselbarth commented 2 years ago
mhesselbarth commented 2 years ago

Fixed in r-spatialecology/shar@75c571b188d03ec82fcb48b6bc5f3ab23fe28063

Bisaloo commented 2 years ago

@lionel68, you didn't check "State of the field: Do the authors describe how this software compares to other commonly-used packages?".

Is this on purpose or did you miss it?

lionel68 commented 2 years ago

My bad, sorry for not checking this thread than often, but seems good to go now! Congrats @mhesselbarth for your careful and patient improvements.

mhesselbarth commented 2 years ago

@lionel68 @tretherington Thanks for all the valuable input!

Bisaloo commented 2 years ago

Great! Thanks @lionel68 and @tretherington for your valuable comments :pray: .

@mhesselbarth, is everything ready on your side? If so, I'll quickly go through everything one last time today and recommend this paper for acceptance. The editor in chief on rotation will take of the rest after.

mhesselbarth commented 2 years ago

@Bisaloo Yes, I would say everything is ready from my side. Should I merge everything from the joss branch to main?

Bisaloo commented 2 years ago

Yes please :+1:, it will probably be easier to review everything

mhesselbarth commented 2 years ago

Okay, main is updated with all changes implemented during the review process. Also contains the paper.md file.

Bisaloo commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

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

Bisaloo commented 2 years ago
mhesselbarth commented 2 years ago

https://github.com/r-spatialecology/shar/commit/e49029242d54503381105fc2594eb95ee511f60c

mhesselbarth commented 2 years ago

All GitHub actions currently fail because https://packagemanager.rstudio.com/ is down (https://github.com/r-lib/actions/issues/447).

Bisaloo commented 2 years ago

What about the reference written as plain text? Yamada et al. 2006

mhesselbarth commented 2 years ago

https://github.com/r-spatialecology/shar/commit/5ba1f84ead10cd15856826700acc665ee4980799 Ups...thanks for pointing out! That completly slipped my mind.