Repository navigation
Standard location for implementation notes? #8930
Description
@astropy/astropy-core-maintainers - quite a while ago (#4821), @astrofrog suggested that it might be good to have implementation notes in some standard place, i.e., notes about the design of a given module. At the time, he considered a separate file, but it was felt that really this should be part of the documentation.
I am reopening this discussion because of #8902, in which @taldcroft adds an _implementation_note docstring to table.py, which has some very useful "developer-beware" notes. I think it would be good to have a standard way to do this, and in particular to ensure such notes become part of the documentation.
My own suggestion is a very simple one: let's use the module docstring for implementation notes. Currently, these are essentially unused (see below for examples from time/core.py and units/quantity.py). Obviously, they will be typeset as part of the API by sphinx, which I think is file.
If we go this route, we could do worse than just start by following the numpydoc guide, which suggests the following sections:
- summary
- extended summary
- routine listings
- see also
- notes
- references
- examples
Examples
# time/core.py
"""
The astropy.time package provides functionality for manipulating times and
dates. Specific emphasis is placed on supporting time scales (e.g. UTC, TAI,
UT1) and time representations (e.g. JD, MJD, ISO 8601) that are used in
astronomy.
"""
# units/quantity.py
"""
This module defines the `Quantity` object, which represents a number with some
associated units. `Quantity` objects support operations like ordinary numbers,
but will deal with unit conversions internally.
"""
Activity
taldcroft commentedon Jun 28, 2019
@mhvk - sounds fine to me. The driver for that ad-hoc note was being able to quickly type stuff right in the same module and then know that myself and others would see it 2 years later. (For table in particular I keep re-discovering subtleties that cannot be ignored when refactoring). From that perspective I like the docstring.
Right now it seems that the module-level docstring is not showing up in the sphinx docs, at least not that I could find. We need to be slightly careful about this if there are a dozen pages of deep developer notes, this should not clutter up user-facing docs.
mhvk commentedon Jun 28, 2019
@taldcroft - yes, indeed, there is no argument that things like your
_implementation_noteare useful, if only for the maintainers themselves!!!On the docstrings showing up: yes, some indeed seem missing. But it actually makes sense: whether or not the docstring shows up depends on whether the particular module is exposed to sphinx. E.g.,
time/core.pyis not, and the fact that one sees nothing in theReference/APIfromtime/index.rstreflects thatastropy/time/__init__.pydoes not have a docstring.But for units, one sees the docstring for
astropy/units/__init__.pyas well as that forquantity.pysince the latter module is also exposed as a module. (I have no idea how the quantity module ended up in front of theastropy.unitspackage...)Back to the actual issue, I think the fact that
__init__.pywill have its own docstring partially addresses your worry about getting too much developer stuff in user-facing documentation: we can and should keep the docstring of__init__.pymuch more generic, leaving detailed implementation notes for the docstrings of sub-modules (which may also be exposed, but that deep down few users will tread!).mhvk commentedon Jun 28, 2019
Getting back to the actual suggestion, I'm not so sure that the list is quite what we want. E.g.,
routine listingis quite pointless, between__all__and automodapi. But perhaps we can interpret it more broadly as a list/description of submodules (in__init__.pyat least).bsipocz commentedon Jun 28, 2019
💯 for the idea, while we need to find the ideal way to implement it (👍 for adding it somehow to the docstrings. I see some hackary way of making sphinx collect certain parts in a different page from the user API ref (e.g. we could have a dev API ref section that filters out and lists only certain methods?). If we ever end up having examples in these docs then having them as proper docstrings also means that they less likely to get out of sync.)
astrofrog commentedon Jun 29, 2019
I'm 100% in favor of a standardized way of doing this but I don't think that using module-level docstrings is the right way to go because I think it would be confusing to have this in the user-facing API docs (for some modules). To some extent, internal implementation is almost irrelevant for the exposed API, and is not something users should worry about. In particular, implementation notes may refer to private API.
Two alternatives:
Or simply comments, with a standard title:
hamogu commentedon Jun 29, 2019
I specifically would like the implementation notes to be user visible. Yes, they need to be marked clearly as "implementation notes, internal details, don't rely on this for use", but I do think it's valuable for advanced users to see why we did something the way we did - we want to break the barrier between advanced users and developers as much as we can.
mhvk commentedon Jun 29, 2019
I'm with @hamogu here, I think it is good for users to see those notes! After all, who knows, someone may get intrigued and go have a closer look at how we do things, and before you know it we have another contributor!
It probably also is good to keep in mind that they will be shown only in the
Reference/APIsections, which are already aimed at more advanced users. It thus would seem the risk of confusion is relatively small.But I also agree that the docstrings should be quite clear what their goal is; looking back at the
numpydocsections, we'd certainly need to replace, e.g., the section title ofnoteswithimplementation notes. And perhaps those should come at the very end, after any examples.Anyway, perhaps the thing to do now is to try it out on some module. Something relatively simple like
timemight be good (although of courseastropy.tableis the reason for this issue).p.s. The other things I really like about using the docstring is that our modules will actually have them. I think we're usually quite good with documentation, but the modules right now are a sad exception.
mhvk commentedon Jul 25, 2019
Reminder partially to me:
coordinates/builtin_frames.pyis a good example of a submodule with a__doc__aimed at developers (though currently it is no longer exposed in sphinx, as__doc__is overwritten by the transform graph; see #9035 (comment) for @eteq's neat solution).eteq commentedon Jul 26, 2019
A bit late to the party, but I also agree this is a good idea. I also fully agree with @hamogu and @mhvk that it's good for users to see this, with one caveat: we want to make sure they know that implementation notes are not necessarily guarantees. That is, implementation notes are good places to say things like "internally this is stored as an X" but we don't want the users to take that to mean "you can write code assuming it is always an X", since in some places we intentionally hiding that in the public API so that we can potentially change in the future. (E.g., places where standard numpy arrays are used but conceivably dask or GPU-accelerated or whatever arrays might work in the future).
Honestly, I'm -0 to it being in module docstrings though. I like the simplicity but see @astrofrog's concern which is similar to mine above. But I think that could be ameliorated by having a standard section at the top that says "warning: these are implementation details that might change, and you might want to ignore them if you are just looking for hte API" or similar.