opnavlab / sonic

SONIC is an object-oriented optical navigation (OPNAV) toolkit written in MATLAB.
MIT License
1 stars 0 forks source link

JOSS review general comments #6

Open Muhao-Chen opened 2 weeks ago

Muhao-Chen commented 2 weeks ago

@athrasher7 Below are my review comments for SONIC openjournals/joss-reviews#6812

  • [x] Please add a setup.m function and use the addpath command to automatically add all the main and subfolders to the main folder.

  • [x] Please add a readme.md to all the folders so that people can read easily what's going on in that folder.

  • [x] Please add a one-sentence description of the licenses to each code file.

  • [ ] Please describe how people can contribute to this software package on the first page. 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.

  • [x] In my personal project TsgFEM I use a MATLAB script called test_all.m (https://github.com/Muhao-Chen/Tensegrity_Finite_Element_Method_TsgFEM/blob/main/Software_Verification_and_Examples/test_all.m) to run all tests/demos automatically. Consider adopting a similar automated testing scheme.

  • [x] Please add more introduction to how to use the software to solve real-world analysis problems on the first page and the paper. It is preferred to have a few bullet points.

  • [x] Other languages are used apart from Matlab and HTML. Consider writing a paragraph to clarify if related software or environment setup is needed.

  • [x] Versions of MATLAB toolbox dependencies (Image Processing Toolbox, Computer Vision System Toolbox) and computer requirements need to be explained in more detail.

athrasher7 commented 1 week ago

In regards to adding more about solving real-world problems, I've added a bullet point list in the readme, but the paper already includes a list in the first paragraph:

"Finally, SONIC offers solutions to the commonly encountered problems of star identification [@mortari2004][@lang2010], triangulation [@henry2023], Horizon-based OPNAV [@christian2021], and more."

These three are very real-world navigation problems. Is there something more you are looking for here? Thanks!

Muhao-Chen commented 1 week ago

Thanks, @athrasher7; the changes look good to me.

athrasher7 commented 6 days ago

Following up on the rest of the items:

  1. SONIC was specifically designed to be used without a set up function. All of the internal folders are '+' folders, which means they are automatically added to the sonic path. The user only needs to add "sonic/" to the path of their working directory, which is a one-liner in MATLAB.
  2. We are in contact with Georgia Tech's OTL about third party contributions. Once we hear back we will include a statement on this.
  3. Thank you for the suggestion, but the SONIC demos are all Matlab live scripts, so a test script to run them wouldn't be intuitive. The user should run these demos locally and then compare the output to the pre-run demos in the API documentation to verify their installation. I've added a sentence to the readme stating this, and to the documentation.
  4. The only languages should now be matlab and markdown. Previously there was more because of python scripts to customize the documentation. For organization, all documentation utilities were moved to a separate branch called Docs.
  5. Version info was added.

Thanks again for the review! I will let you know when we add more info on third party contributions.