JOSS Review #6025 CRNPy

[jlarsen-usgs]

This issue contains my review for the JOSS submission openjournals/joss-reviews#6025 :

You can either address this as a single issue or spawn a new issue from each bullet point

Paper:
Reviews for the paper are including in the attached pdf 10.21105.joss.05814_jl.pdf. Of note:

• The statement of need could be better stated. Correction and filtering methods documented in many different papers is fairly common. I suggest reframing the statement of need with regard to reproducibility and robustness.
• References in the paper are missing DOIs

Software:

• Unversion the crnpy.egg-info folder from the repository. This is a folder that pip creates at installation on a local machine and shouldn't be needed in the repo.
• Readme.md: Include a section/link to the example problems and maybe a short description of each example problem
• Testing: It would be nice to see testing of each/most of the individual methods to make sure that the code is giving you the expected output from synthetic or literature test cases. This will also make tracking down and fixing code errors in the future much easier.
• A type checker/conversion method should be developed and used with your processing methods. This would also help with long term maintainability of the code. I noticed in multiple methods, docstrings indicate that numpy, list, or pandas could be supplied as inputs, however there were pandas specific methods in the processing that would fail with a numpy array or a list.
• total_row_counts() method has unused input parameters. These unused input parameters should be removed from the method.
• correction_pressure(): update the docstrings. Docstrings list atm_pressure as an input parameter, however the parameter name is pressure.
• correction_humidity(): update the docstrings. temp is listed as an input parameter although it is not currently one.
• get_incoming_flux(): unused date_format variable should be removed
• smooth_1d(): Need to add the proper reference for the Savitzky-Golay filtering method. "Savitzky and Golay (1964)".
• sensing_depth(): remove trailing semicolons
• find_neutron_monitor(): verbose parameter is missing from docstrings
• lat_lon_to_utm(): This method could and probably should be removed. The python package utm does the same thing, is robust, and does not have unnecessary dependencies.

Optional but highly recommended:

• Lint the code for readability and uniformity. I personally like using black for this, but there are many python linters that will do the job

[andres-patrignani]

I accidentally created a new issue by clicking on the last point and in an attempt to undo this action I unwillingly deleted the issue made by the reviewer. The original suggestion made by the reviewer was: "Lint the code for readability and uniformity. I personally like using black for this, but there are many python linters that will do the job."

[joaquinperaza]

@jlarsen-usgs Thanks for your valuable feedback! I updated the library software in the previous commits to address your suggestions.

[joaquinperaza]

A series of commits were made to address the feedback comprehensively. The crnpy.egg-info folder was unversioned from the repository, aligning with best practices for Python package repositories. The README.md was updated to include links and descriptions of example problems, enhancing the documentation for users. Extensive testing was added, covering individual methods to ensure they produce expected outputs, which aids in future debugging and code maintenance. A type checker/conversion method was implemented to ensure robust handling of different input types, addressing the issue of method failures due to incompatible input types.

Unused input parameters in methods like total_row_counts() were removed, streamlining the code. Docstrings for methods such as correction_pressure() and correction_humidity() were updated for accuracy. The unused date_format variable in the get_incoming_flux() method was removed, and the proper reference for the Savitzky-Golay filtering method was added to smooth_1d(), adhering to academic standards. Trailing semicolons were removed from sensing_depth(), and the missing verbose parameter was added to the docstrings of find_neutron_monitor(). The lat_lon_to_utm() method was refined, considering the availability of the robust utm Python package, which serves a similar purpose.

The code was formatted to increase readability, and the statement of need was updated considering changes also from issue #10, References and DOI's were fixed together with comments from #8

[elbeejay]

@jlarsen-usgs if you could take another look and weigh in when you get a chance that'd be great, thanks!

[elbeejay]

@jlarsen-usgs - let us know when you'll have a chance to review these changes

[elbeejay]

Per openjournals/joss-reviews#6025 (comment) - @jlarsen-usgs has indicated that the suggestions made in this issue have been resolved.