DM-17831: Rewrite docstrings in numpydoc format #66
Conversation
Ensure that max-doc-length <= 79
builders.py and installation.py have problems due to the memberOf decorator.
timj
force-pushed
the
tickets/DM-17831
branch
from
02d7760
to
bc0ad13
Compare
|
I've added some Sphinx building. The main remaining problem seems to be that the |
This allows the memberOf methods that have been monkey patched into the base SConsEnvironment to have their documentation displayed.
There was a problem hiding this comment.
This is brilliant 🏆 Thank you @timj!
I don't have much to add, and I think this definitely adds a lot of value and future potential and we should add this to the pipelines_lsst_io build.
It might be useful to have some explainer after the first paragraph on the index.rst saying that the SConsUtilsEnvironment collects all the methods?/functions? available in the SCons environment, but is itself a construct for the purpose of the documentation. Up to you, if you can word it (I don't understand it enough at the moment).
There's also a fix to make in scripts.py, but that's it.
python/lsst/sconsUtils/scripts.py
Outdated
| as independent single tests. By default this list is empty | ||
| and all tests are run in a single pytest call. | ||
| Items specified here will not appear in the default pyList | ||
| and should not start with "test_" (such that they will not |
There was a problem hiding this comment.
Believe it or not, this line is generating this warning:
scripts.py:docstring of lsst.sconsUtils.scripts.BasicSConscript.tests:44: WARNING: Unknown target name: "test".
(that is, it wants to make test_ a link.
Luckily it can be fixed by making it a literal:
``"test_"``There was a problem hiding this comment.
Thank you! That warning was bugging me and I couldn't work out what line it was talking about.
doc/conf.py
Outdated
|
|
||
| """Sphinx configuration file for an LSST stack package. | ||
|
|
||
| This configuration only affects single-package Sphinx documenation builds. |
There was a problem hiding this comment.
"documenation" → "documentation"
doc/lsst.sconsUtils/index.rst
Outdated
| lsst.sconsUtils | ||
| ############### | ||
|
|
||
| The ``sconsUtils`` module provides tooling to support the building of standard |
doc/lsst.sconsUtils/index.rst
Outdated
|
|
||
| Many of the methods defined in this package are injected directly into | ||
| the base SConsEnvironment. | ||
| The `~lsst.sconsUtils.installation.SConsUtilsEnvironment` class solely exists to allow these methods to be documented. |
There was a problem hiding this comment.
"solely exists" → "exists solely"; maybe make this a .. note::?
python/lsst/sconsUtils/builders.py
Outdated
|
|
||
| Files are chosen if they match fileRegex; toplevel directories in list | ||
| ignoreDirs are ignored. | ||
| This routine won't do anything unless you specified a "TAGS" target |
| The name of the calling .cfg file, usually just passed in with the | ||
| special variable ``__file__``. This will be parsed to extract the | ||
| package name and root. | ||
| headers : `list`, optional |
There was a problem hiding this comment.
'list' of 'str' (with backquotes)?
| Returns | ||
| ------- | ||
| result : `bool` | ||
| Did the flag work? |
There was a problem hiding this comment.
"flag"? Looks like this was copied from the function above. Same goes for the next function.
|
|
||
| Notes | ||
| ----- | ||
| .. code-block:: python |
There was a problem hiding this comment.
Add a line of text saying at least "Sample usage:"?
python/lsst/sconsUtils/tests.py
Outdated
| with glob patterns. If a file is listed as "@fileName", the @ is | ||
| stripped and we don't bother to check if fileName exists (useful for | ||
| machine-generated files). | ||
| expectedFalures : `dict`, optional |
There was a problem hiding this comment.
"expectedFalures" → "expectedFailures"
| Notes | ||
| ----- | ||
|
|
||
| .. code-block:: python |
python/lsst/sconsUtils/utils.py
Outdated
| Returns | ||
| ------- | ||
| hasSIP : `bool` | ||
| `True` if SIP is present in this operationing system version. |
There was a problem hiding this comment.
"operationing" → "operating"
Still needs a sphinx directory.