openjournals / joss-reviews

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

[REVIEW]: Circular profile of a ring over a stack of images #3426

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: @jeanbilheux (Jean Bilheux) Repository: https://github.com/neutronimaging/circular_profiler Version: v1.0.0 Editor: @jgostick Reviewers: @michaelberks, @jgostick Archive: Pending

: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/5efb319c2afb26d71f1dd6e46f81c5cb"><img src="https://joss.theoj.org/papers/5efb319c2afb26d71f1dd6e46f81c5cb/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/5efb319c2afb26d71f1dd6e46f81c5cb/status.svg)](https://joss.theoj.org/papers/5efb319c2afb26d71f1dd6e46f81c5cb)

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

@prasantapal & @michaelberks, 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 @jgostick 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 @michaelberks

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. @prasantapal, @michaelberks 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
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/MCSE.2011.37 is OK
- 10.21105/joss.00815 is OK
- 10.1109/MCSE.2007.55 is OK

MISSING DOIs

- 10.25080/majora-92bf1922-00a may be a valid DOI for title:  Data Structures for Statistical Computing in Python 

INVALID DOIs

- None
whedon commented 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.05 s (484.5 files/s, 90993.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          16            510            181           1851
Qt                               1              0              0           1433
Markdown                         3             37              0            131
YAML                             2              7              1            112
CSS                              1             21             10             96
TeX                              1              4              0             45
Bourne Shell                     1              5              5             24
Jupyter Notebook                 1              0            397             13
-------------------------------------------------------------------------------
SUM:                            26            584            594           3705
-------------------------------------------------------------------------------

Statistical information for the repository 'a24e99000924813bd6177714' was
gathered on 2021/06/28.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
JeanBilheux                      4          2544              2          100.00

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
JeanBilheux                2542           99.9          0.0                5.35
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:

jgostick commented 3 years ago

Hello @michaelberks and @prasantapal, the review is now under way. You can find a checklist at the top of this issue thread, one for each of you. Your task is simply to check off all those boxes. This mostly involves installing the software according to the provided documentation, and verifying it's functionality. If you find any boxes that you cannot check, then you let the author(s) of the package know and they should address it. You can ping them through this issue, or by opening an issue on their github repo directly.

Thank you both very much for your time.

whedon commented 3 years ago

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

whedon commented 3 years ago

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

jgostick commented 3 years ago

@prasantapal and @michaelberks, I have just sent you emails to remind you about this review. Please let me know asap if you're no longer able to do this review, thanks.

michaelberks commented 3 years ago

@jgostick apologies for not starting the review earlier. I am doing this today.

michaelberks commented 3 years ago

@JeanBilheux my initial review comments in italics:

Functionality

Installation:

Does installation proceed as outlined in the documentation? I have tried installing the software but received an error running the condo environment setup script. I have logged a new issue in the source code repo. I am unable to proceed with reviewing functionality.

Documentation

Example usage:

Do the authors include examples of how to use the software (ideally to solve real-world analysis problems). The software would benefit from including a small test dataset of images, with specific description of how to run the GUI tool on this data

Functionality documentation:

Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)? There is minimal documentation, however I get the impression the tool is supposed to be simple and intuitive to use once the GUI is running. Because I could not get the GUI to run (see installation error above) I cannot confirm if this is the case though.

Automated tests:

Are there automated tests or manual steps described so that the functionality of the software can be verified? There is a tests folder in the repo, and the last commit comment mentions starting to implement unit tests, but it is not clear if this is complete, and no instructions are given for running the tests

Community guidelines:

Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support I could not find these guidelines

Software paper

A statement of need:

Does the paper have a section titled 'Statement of Need' that clearly states what problems the software is designed to solve and who the target audience is? I think the statement of need could be clearer in how this applies more widely to the target audience. At first read it appears to solve a very niche problem, which may only be applicable to the authors' own lab

State of the field:

Do the authors describe how this software compares to other commonly-used packages? No

Quality of writing:

Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)? The summary is very clear. I think the statement of need requires some work though. It isn't clear in sentence 2 who the users are or what the sample is - a real physical object or the data files from sentence 1. Then in sentence 3, what is the large volume of data? Is all of this just referring the fact that the output of a CT scan is a set of image slices (stored in TIFF or FITS format)? What specific type of analysis is time consuming? What type of symmetry is required of the sample? The proposed software achieves a very specific type of analysis, and it isn't clear what specific properties of the sample being imaged need to be to make this software applicable. It is then mentioned this has been used on equipment in the authors' lab, to analyse a specific fault. It isn't clear to me from the rest of the statement of need that this software generalises to other problems. By that I mean, clearly code that computes intensity profiles in a ring across a stack of images can be applied to any set of images, but it isn't clear why these measurements would be useful in other datasets.

References:

Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax? No references are given, this again makes me slightly concerned about the generalisability of the software to other existing problems, or whether it is of use to other researches

JeanBilheux commented 3 years ago

Thanks for the feedback. I will work on all those comments once I'm back from vacation, in 2 weeks. Thank you

JeanBilheux commented 3 years ago

Installation I fixed the error in the installation that seems to occur for some user (having an older version of nodejs already installed.

Example usage I added a set of demo images to illustrate how to use the notebook and step by step instructions on how to use them in the README (https://github.com/neutronimaging/circular_profiler/issues/3)

JeanBilheux commented 3 years ago

Functionality documentation

I added a quick tutorial in the README but a more detail instruction can be found here as mentioned at the top of the README.

JeanBilheux commented 3 years ago

Automated tests

I added instructions on how to run the unit tests and fixed the important warnings. https://github.com/neutronimaging/circular_profiler/issues/4

JeanBilheux commented 3 years ago

I added some comments about Community Guidelines in the README.

JeanBilheux commented 3 years ago

Statement of need

Rewrote it by using a more general example where this tool can be used.

JeanBilheux commented 3 years ago

State of the field: I can not compare this software to other packages as to my knowledge, there isn't any. References: Data used in this study are protected. Algorithm was developed by owner of this code.

JeanBilheux commented 3 years ago

@michaelberks Thanks Michael for all your comments. I think I answered all of them. Feel free to check again.

JeanBilheux commented 3 years ago

@michaelberks Any updates on this review?

michaelberks commented 3 years ago

@JeanBilheux apologies for not get back to you sooner. It's going to be very difficult to do anything before Nov 8th as there's a major deadline in my field to complete work for. I promise to make this a priority as soon as that is out the way though.

JeanBilheux commented 3 years ago

it's ok @michaelberks. I know how things are crazy this year. Thanks for the update.

jgostick commented 3 years ago

Hi All, I also appologize for my absence. No excuse. I have pinged @prasantapal several times via email and he's awol, so I will need to find another reviewer. I am thinking that I might do it myself given how long this submission has been sitting, and it also happens to be right up my alley.

jgostick commented 3 years ago

@whedon remove @prasantapal as reviewer

whedon commented 3 years ago

OK, @prasantapal is no longer a reviewer

jgostick commented 3 years ago

@whedon add @jgostick as reviewer

whedon commented 3 years ago

OK, @jgostick is now a reviewer

jgostick commented 3 years ago

Review checklist for @jgostick

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

jgostick commented 3 years ago

Hi @JeanBilheux..firstly, I am very sorry about how long this has taken! I have dedicated this afternoon to doing the review of your package, and have run into some issues during installation and usage:

jgostick commented 3 years ago

Since I was unable to run it, I thought I would take a look at some of other checkboxes:

import numpy as np
import matplotlib.pyplot as plt
from tqdm import tqdm

# %% User input
im = np.random.rand(500, 500, 500) > 0.8  # The image to be analyzed

# User specified ring properties
c = [240, 250]  # center
ri, ro = (100, 150)  # inner and outer radii
n = 50  # Number of angular bins to use

# %% The code
# Generate a ring
xx, yy = np.meshgrid(range(im.shape[0]), range(im.shape[1]))
d = np.sqrt((xx - c[0])**2 + (yy - c[1])**2)
ring = (d > ri) * (d < ro)
# Make it a 3D annulus
annulus = np.tile(np.atleast_3d(ring), [1, 1, im.shape[2]])
# Create theta field for thresholding below
q = np.rad2deg(np.arctan((yy - c[1])/(xx - c[0])))
q[xx < c[0]] = q[xx < c[0]] + 180
q += 90

# Scan image for specified number of angles
bins = np.linspace(0, 360, n, endpoint=False)
vals = np.zeros_like(bins)
data = im*annulus
w = 360/n
for i, a in tqdm(enumerate(bins)):
    mask = (q > a)*(q < (a+w))*annulus
    vals[i] = data[mask].sum()/data[mask].size

# %%  Do some plotting
fig, ax = plt.subplots(1, 4)
ax[0].imshow(im[..., 50], origin='lower')
ax[1].imshow(ring, origin='lower')
ax[2].imshow(data[..., 50], origin='lower')
ax[3].plot(bins, vals, 'b-o')
ax[3].plot([0, 360], [np.mean(vals)]*2)
plt.ylim([0, 1])
bidochon commented 3 years ago

So for now, I'm going to retract this publication for the following reasons:

jgostick commented 3 years ago

I am sorry that I chased you away. I am a JOSS associate editor, but I do not speak for JOSS officially...I just sheppard along the review process (in your case by acting as the second reviewer). However, JOSS editors tend to decline GUI-only type contributions (like the Shiny apps written for R) during the "scope check" phase (which your package passed). These usually represent wrappers around other code, and don't contribute much/any core functionality. When I think about whether a package is of appropriate "scope" I usually ask myself, "would a user of this software feel the need to cite it in their actual research paper". To me this means "does this software package do something fundamental enough that an author feels the need to point the reader to a source explaining its function". By this definition, a GUI-only software is not sufficient. Instead an author would point to the paper describing the underlying functionality that the GUI wraps since that's the code that actually touched/manipluated/produced the data. Am I making sense?

In your case, you DO offer some core functionality, so it did make it past the scopy check. However, I was curious how much functionality was actually required to accomplish this, so wrote the above function (also for fun). After this excerise I am not not 100% convinced that your software offers sufficient functionality. On the other hand, the mission of JOSS is to provide software developers with a means of getting recognition for their time and efforts. I am wiling to continue this review if you are still intersted. A "quick and effortless install on all possible platforms" is not a requirement for JOSS so I can try to install it on the Ubuntu box in my lab. I had hoped that using WSL2 would work (it does for a lot things, even OpenFOAM), but I guess your GUI requirements implicitly depend on some of the core components (probably related to the desktop UI?).

All that being said, I feel like your submission still requires quite a lot of work. Many of the 'check boxes' are not satisfied and overall your submission feels incomplete. So, I think that a temporary withdrawal of the submission makes sense, and you can resubmit later once things are more polished.

Here are some suggestions:

jgostick commented 3 years ago

@whedon assign @jgostick as editor

danielskatz commented 2 years ago

@whedon withdraw

whedon commented 2 years ago

Paper withdrawn.