[WIP] Update API documentation

[weaverba137]

This PR updates the API documentation file doc/api.rst and fixes errors introduced by adding documentation of docstrings that had not previously been tested.

This PR has many changes, but I promise they are only to the documentation, not to the code.

• Create a CI test that ensures that doc/api.rst is up-to-date with the actual *.py files in the package.
• Check that every *.py file has at least a module-level docstring containing the name of the module.
• @sbailey: these files need to be examined to see if the code can be simply wrapped in a main() function: py/desispec/desi_bias_dark_1d_model.py, py/desispec/desi_create_bias_dark.py. These are the only files that have this property. All other desispec files can be imported by Sphinx successfully.
• Clean up indentation errors reported when compiling Sphinx. Most other documentation error types are already cleaned up.
• Stretch goal: check that doc/changes.rst is up-to-date.

[sbailey]

@sbailey: these files need to be examined to see if the code can be simply wrapped in a main() function: py/desispec/desi_bias_dark_1d_model.py, py/desispec/desi_create_bias_dark.py. These are the only files that have this property. All other desispec files can be imported by Sphinx successfully.

Those can be wrapped in if __name__ == '__main__:' blocks, or moved into a main() function that is called by if __name__ == '__main__': main(). I think they could even be completely removed (they have been superseded by other scripts in bin/), but I'd rather not remove code at all mid-stream during the Iron production (we'll make another tag at the end to include any doc changes or downstream fixes for steps that haven't run yet, e.g. #1930). So go ahead and wrap in main / if __name__ ... and we can consider completely removing during later spring cleaning (along with other unused code).

Check that every *.py file has at least a module-level docstring containing the name of the module.

Thanks for adding those, but let's not include documentation notes-to-self like "Please add module-level documentation." under the titles. For cases where we don't have the time/knowledge to add them to this PR, I'd prefer to leave it blank so that we don't propagate notes like that to readthedocs.io. That kind of note would be better tracked as a cleanup ticket.

This PR has many changes, but I promise they are only to the documentation, not to the code.

Editorial: this also has lots of trailing-whitespace cleanup in the code itself, which in the past has led to confusing merge conflicts when one branch makes whitespace changes and another simultaneous branch makes algorithmically substantive changes. So in general I'm not a fan of post-facto auto-removing trailing whitespace unrelated to the core changes of a PR. However, in this case, you've already made and committed all those white space changes and we might be in as clean a state as we're ever going to get for minimizing the number of active development branches that could conflict with it, so let's go for it.

[weaverba137]

OK, I'll remove 'Please add' lines (there are a lot of them though!).

The thing about removing trailing whitespace is that it's hard to turn off on a package-by-package basis, so there will always be someone who has it turned on, and you just have to deal with it.

Astropy is moving toward extremely strict enforcement of style using the black package. The idea being, no matter what style you use, black forcibly changes the style, with no real options. I'm sure automatic removal of trailing whitespace is one of the features. So if we don't want to be that strict, there will always be cases where someone adds whitespace and another person removes it.

[weaverba137]

@sbailey, I think this will be ready to merge after tests pass.

[weaverba137]

Before I forget: the API completeness test that I added to CI relies on a branch of desiutil, related to desihub/desiutil#190. It would be nice if we could merge that PR so that we can run the test off a tag of desiutil. If you don't want to interfere with iron at this point, just don't install that tag until it is done.

[sbailey]

Before I forget: the API completeness test that I added to CI relies on a branch of desiutil, related to desihub/desiutil#190. It would be nice if we could merge that PR so that we can run the test off a tag of desiutil. If you don't want to interfere with iron at this point, just don't install that tag until it is done.

This is a chicken-vs-egg problem: We were purposefully holding off on merging desihub/desiutil#190 because that would break how Iron tags were installed, but we also wanted to get these desispec docs in shape for the final Iron wrap-up tag, so I don't want to create an iron-era new version of desispec that requires that new version of desiutil. What's the specific piece of the new unit test that requires the new desiutil branch? It looks like it might just be whether we call "desi_data_census" (current) vs "desi_api_file" (new). Or does it go deeper than that?

[sbailey]

Ben and I chatted about merging the desiutil branch or not. Merging it would require some updates to other packages too to maintain full compatibility between the "main" branch of all packages, and to maintain compatibility between the latest tags of all packages. So for now, we're not going to merge that branch of desiutil, and the github actions api.rst test will continue to use the desiutil branch. After Iron is over, we can update all the packages together, including switching this github actions test to use the new desiutil tag. It is admittedly somewhat distasteful to have a github test rely upon a branch of another package, but this doesn't impact the normal unit tests and I think this is better than violating the rule "main of all packages should work together" and "the latest tag of all packages should work together".

I'm going to cross my fingers and merge now before this PR reaches 200 changed files. :)