New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REVIEW]: atems: Analysis tools for TEM images of carbonaceous particles #6416
Comments
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
Wordcount for |
|
Review checklist for @jonbmartinConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
I have completed my review and I am satisfied with both the software toolbox and the package describing it, and believe they are appropriate for publication in JOSS. The MATLAB functions seem robust - I tried to add a few different types of noise to the images before segmentation and processing and got what seemed like satisfactory results every time. The GUI interface (documented in main_0.m) built as a MATLAB app is very nice and clean, and a flexible addition to the automatic tools. However it is a little bit difficult to use. I found myself struggling to use the slider to specify 'threshold relative to Otsu' - allowing users to input a specific number than rely on a slider could help a bit. I also had some trouble with tools.load_imgs(), I believe as a linux user. I was able to run the command without arguments succesfully, but could never get it to succesfully accept a string input as the file path. Wasn't able to fully troubleshoot. The paper is very well written, as is the documentation that was developed. I think that overall this is an excellent product and highly recommend publication. |
@jonbmartin thank you very much! @tsipkens, the two points raised (the somewhat finicky slider, and the question about running tools.load_imgs() with a string input) seem fairly minor, but do you have any thoughts in response? |
Yes, I will! Hopefully I can get around to posting them shortly. |
Review checklist for @tytellConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@diazrenata. Some changes have been made to the code in response to @jonbmartin's recommendations.
The GUI was amended accordingly, adding a numerical field in addition to the slider.
It is unclear as to the source of this issue. There is a reasonable chance this is related to the use of Linux. The CircleCI workflow still compiles, though this uses a weblink as an input. A note about potential issues for Linux users has been added to the README. The load_imgs function was also amended to take file names in addition to a folder, which could present another potential source for error. |
@diazrenata. I have also addressed the issue that @tytell raised against the repository, namely an error in the README that did not match one of the sample scripts. |
I have completed my review. As far as I can tell, the software does what it claims to do. My expertise is mainly in image analysis, and so I cannot address the specifics of analysis as it relates to soot or carbonaceous particles. The analysis is based on what seems to be a community need and seems based in existing literature. The paper is clear and well written and the scripts are documented and seem to run as described. Here are a few notes and comments. I've added a few more minor issues to the repository.
I think this is a Matlab version incompatibility. Here is my Matlab version info:
|
@tytell Thanks for the thorough review. I will work to address some of these issues at my earliest convenience. |
@tsipkens 👋 Just checking in to see how revisions are going! |
I made a bunch more progress today, I hope to finish with a larger response soon. Thank you for the reminder. |
Thanks to @tytell for the useful feedback. Brief responses to the review below:
All corresponding issues against the repository have been closed with comments. |
Submitting author: @tsipkens (Timothy A. Sipkens)
Repository: https://github.com/tsipkens/atems
Branch with paper.md (empty if default branch):
Version: v1.2
Editor: @diazrenata
Reviewers: @jonbmartin, @tytell
Archive: Pending
Status
Status badge code:
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
@jonbmartin & @tytell, 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:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @diazrenata 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 @jonbmartin
📝 Checklist for @tytell
The text was updated successfully, but these errors were encountered: