ihesp / IPART

Image-Process based Atmospheric River Tracking (IPART) algorithms
https://ipart.readthedocs.io/en/latest/
GNU General Public License v3.0
24 stars 8 forks source link

API documentation: undocumented parameters & methods #3

Closed sadielbartholomew closed 4 years ago

sadielbartholomew commented 4 years ago

Hi. Though the API documentation is quite detailed, towards the openjournals/joss-reviews#2407 review, I have noticed two aspects that I think are barriers towards the 'Documentation -> Functionality documentation' review requirement being deemed satisfactory:

  1. There are some parameters for a number of methods in the IPART API that are not documented (see examples below);
  2. Not all of the util modules, and not all methods from those that are present, are documented. This is indicated via '(selected parts)' after the names of the utils.funcs.py & utils.plot.py pages in the page listings, but there is no explanation as to why the modules under utils in the directory hierarchy are only partially documented.

Examples for (1)

For example (though it may be the case for other modules so please check), looking at the API reference pages for thr.py and AR_detector.py I notice that:

Resolution

Please can you address the following points to resolve the above:

Thanks.

Xunius commented 4 years ago

@sadielbartholomew Thanks for the comments.

Please checkout the commit and this commit addressing these issues.

Regarding the documentation for other functions in utils.func.py and utils.plot.py, I added doc strings for them in another branch (netcdf), but I was not advised to merge until review is finished, as I made a dependency change in that branch.

sadielbartholomew commented 4 years ago

Thanks @Xunius. I will get back to you later today about this, & to continue my reviewing, but from what I can see from a quick look that all sounds good & I can likely close the Issue later on after the new commits you have made.

kbarnhart commented 4 years ago

~Linking this to the JOSS review thread: openjournals/joss-reviews/issues/2407~ Apologies. I now see that @sadielbartholomew already did this.

sadielbartholomew commented 4 years ago

Thanks @Xunius, I have checked the status of the documentation again and you have addressed my feedback well with those new commits & others, so I can close this. I am happy to tick the 'Functionality documentation' criteria off the checklist, especially given the new detailed sections for each of the four core functionalities (the 'Perform the THR computation on IVT data' page, etc).