add interface documentation #284
Conversation
There was a problem hiding this comment.
This looks good. I think it's a very good start.
The minus is that the documentation is not fully automated. A user has to run the script to generate the updated documentation when needed.
It would be relatively easy to write a pre-commit hook that automatically runs the generate_interfaces.sh script before each commit. Hooks have to be installed manually by the developer though, for security purposes, so there is still an initial action needed, although we can document that in our Developer documentation.
This does not meet a separate objection which might be to generate a fully interactive copy of the code on the web.
If we go this way I think we should use something that already exists. Also, the code is already on GitHub, so it is browsable already. If by "interactive" you mean that one could eg. click on a module name in a use statement, or a subroutine name in a call statement to be taken automatically to the file that defines this module/subroutine, that would indeed be useful (and cool!). GitHub is already adding support for that but it's focused on more modern langages at the moment. The underlying infrastructure is open-source, so it may support Fortran one day (one can always wish...)
doc/generate_interfaces.sh
Outdated
| echo " " >> $rstfile | ||
| echo "${title}" >> $rstfile | ||
| echo "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" >> $rstfile | ||
| echo "::" >> $rstfile |
There was a problem hiding this comment.
Changing :: to .. code-block:: fortran will make Sphinx syntax highlight the code blocks. I tried it locally and it looks great.
I think this would be nice.
There was a problem hiding this comment.
I replied a couple days ago, but maybe didn't push the button. This is a great recommendation and I have implemented it. Thanks!
doc/generate_interfaces.sh
Outdated
| while IFS= read -r line; do | ||
| if [[ $line =~ .*$endline.* ]]; then | ||
| dowrite=0 | ||
| #echo "$dowrite $line" |
There was a problem hiding this comment.
do we want to keep those commented lines ?
There was a problem hiding this comment.
I have removed the debug lines. I tend to leave them in because they always seem to come in handy later, but understand it's nice to leave it in a cleaner state.
|
Regarding automation of documentation generation. Webhooks might work. I was actually thinking about adding an option to cice.build or cice.setup to run the interface documentation script. It could even be done "under the covers" whenever either are run, although I think I prefer an explicit command to trigger it for now. I think the main thing is that we probably won't need to run it too often, so having it done automatically all the time seems a little risky and maybe expensive (think about large test suites although we could turn it off on test suites). Maybe what I'll do is add a new option to cice.setup to support running the documentation script. Then if we decide it should be done automatically, all the time, we could make that happen. Even if cice.setup update the local rst file automatically, it still requires a user to add and commit the change to the rst file in the repo manually. But I guess by running the script automatically, it would highlight times that the file does need to be updated. I'm open on strategy. |
|
Just to clarify: I was referring to Git hooks, which are scripts that run in the local repo. |
|
I had a look at some githook pre-commit documentation. It looks very cool. It's too bad they can't be added to a repo automatically though. As @phil-blain points out, that creates some extra work. I think I'll just create an option in cice.setup to run the script for now and we can evolve that in the future as we need to. |
|
I added a --docintfc option to icepack.setup to make updating the icepack public interface documentation a little more straight-forward. I have also updated the documentation to reflect that. Going back to the full web browsable source code implementation, I agree we should use a tool to do that. I have used doxygen in the past, but there are others. I think we should decide first whether we want to provide that to the community. I personally don't use those kinds of browsing tools, but that doesn't mean we shouldn't do it. I agree with @phil-blain that it would be great if github provided some of the click-thru capability on the site itself as the code is already there. |
|
@duvivier you can see the read-the-docs build by clicking "Show all checks", then "Details" next to the read-the-docs item. |
|
@phil-blain I apparently totally blanked about our new and wonderful functionality. (face-palm). I guess that means I'm tired. It looks great. |
|
@phil-blain thanks for all your feedback. Since you provided some comments before, I'd like to confirm you are comfortable with this PR and we could merge. I'm also happy to consider further updates if you have suggestions. thanks! |
PR checklist
Add interface documentation and script to generate those interfaces
tcraig
No code was changed, but I will run some tests shortly
This is related to issue #283. After messing around with sphinx-fortran for many hours, and running into many issues, I decided to implement a short script to extract the public interfaces and add them to the rst documentation. The pluses for using this method are that we control the implementation and it's simple. The minus is that the documentation is not fully automated. A user has to run the script to generate the updated documentation when needed. The problems with sphinx-fortran will be documented in #283 further. This could be just an interim solution.
This meets my primary objective which is to document the public icepack interfaces in the user guide. This does not meet a separate objection which might be to generate a fully interactive copy of the code on the web.
As part of this documentation, we should continue to improve the inline documentation in the public interfaces in the source code, making the subroutine purpose and arguments clearer. Separately, I think we still need to documented the calling sequence for using icepack more generally. There is a place holder in the current documentation for that.
Please see section 3.5 in readthedocs for the latest updates in the documentation including the interfaces.