Auto-generate API reference in docs and an overhaul #430
Conversation
Codecov Report
@@ Coverage Diff @@
## master #430 +/- ##
==========================================
+ Coverage 91.31% 91.45% +0.14%
==========================================
Files 58 60 +2
Lines 2705 2751 +46
==========================================
+ Hits 2470 2516 +46
Misses 235 235
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
CasperWA
changed the title
| for filename in filenames: | ||
| if re.match(r".*\.py$", filename) is None or filename == "__init__.py": | ||
| # Not a Python file: We don't care about it! | ||
| # Or filename is `__init__.py`: We don't want it! |
There was a problem hiding this comment.
Do we not want to capture e.g. module-level docstrings (that would be in the __init__.py)?
There was a problem hiding this comment.
The issue is that it doesn't come off nicely in the API Reference section, since it will "break" the system a little. One will have to have a __init__ entry in the ToC to work this in. I have tried here to avoid that, but it may be that it's still better than nothing?
There was a problem hiding this comment.
Yeah I get you, not sure the best way of fixing it really with the current tree structure we have in the docs. Unless it could be renamed to "contents" and include the submodule docstrings... probably more trouble than its worth.
There was a problem hiding this comment.
Yeah, but not a bad idea. I'll give it a go.
And then make sure to also name the ToC header something non-API-like, e.g., "Contents" as you suggest (the capitalization being important here).
There was a problem hiding this comment.
So mostly this presents the issue that mkdocstrings cannot handle the __all__ "imports". I.e., it errors when trying to build the docs due to a "NameError":
__all__ = exceptions.__all__ + references.__all__ + structures.__all__ # noqa: F405
NameError: name 'exceptions' is not definedThere was a problem hiding this comment.
This is also why I gave up and moved the resource-specific adapters I had written in the __init__.py files to new adapters.py files, so they could be included.
There was a problem hiding this comment.
But I could instead try to create a custom markdown file that already exists at the top of each module that can parse in the doc string from the __init__.py file? Unfortunately, it won't get anything else from there. Like, e.g., the validate() function in optimade.validator.__init__
There was a problem hiding this comment.
So if you want to include this, I suggest instead to move it to a new file, e.g., cli.py and keep __init__.py only to create an __all__ if needed?
|
Okay, I may have gone a little overboard now (for this PR). But I have now updated all doc strings relevant for the |
|
It seems the only "issue" is the type annotation for the |
|
Wow... I walk away for a few weeks and you rewrite all the docs. :P |
|
I'll just wait for a response from @ml-evs before merging, since he has reviewed this a couple of times already :) |
There was a problem hiding this comment.
It seems the only "issue" is the type annotation for the
pymatgenconversion function, since it extrapolates the complete type path, which is wrong for the customly created typesStructureandMolecule.
I don't consider it a huge issue.
What do you think @ml-evs?
I think it's fine, and already a big improvement on "None".
This PR is really great, thanks for all your hard work @CasperWA, it has been fun vicariously learning mkdocs through you... I think my only slight niggle is that lack of a top-level module list (with the module docstrings giving an overview), but that's something we can work on in the future if we get time!
I have never used Concerning your last point here. You could make an issue for it (low priority) and we can get back to it later/you can try stuff out locally to see if you get a nice thing going? 😃 |
Update icons and logo. Use new color scheme. Rename INSTALL.md -> install.md Added GitHub social in site footer pointing to Materials-Consortia. Fix existing links by creating symlinks. Add PyMdown Snippets: https://facelessuser.github.io/pymdown-extensions/extensions/snippets/
This is an example update of how the doc strings should be written to show up nicely in the new API reference documentation.
This is done to have easy access to any configurations should we want it. It also helps to show what the default is right away, instead of having to check the mkdocstrings documentation. Extend filters to exclude `__all__` and `__config__`. Add to the invoke task that for `models` only, it should set `rendering.show_if_no_docstring` to True. This ensures we get _all_ models into the documentation, no matter if the have doc strings or not.
It correctly added `optimade.server.warnings`. Prior to which `mkdocs build` emitted warnings about missing cross-references (to `OptimadeWarning`). Afterwards all was beautiful and quiet :)
Update malformed cross-references after rebasing. Move the attributes `provider_prefix` and `provider_fields` from `MongoCollection` to the parent abstract class `EntryCollection`.
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
Also add doc string for `EntryResource`.
The class doc string is not parsed correctly by `mkdocstrings` if a line break is not included straight after the first `"""`. For all other doc strings there does not seem to be a similar issue. Hence they have been reverted to keep their original form where possible. Furthermore, all `Returns` has had their "type"-annotation removed, since this is retrieved from the Python type annotation by `mkdocstrings` instead.
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
The type annotation can be left out of Parameters lists, since they will be taken from the Python type annotation.
The code is lifted from the `mkdocstrings` repository's own documentation. It indents the references according to subsections and creates a vertical gray line left of the text in the indentation. Also, it makes sure not to break code weirdly. Furthermore, the starting heading level is reset to the default (2). And the `optimade` folder (and all sub-folders) are added to the watch list for mkdocstrings to request a rebuild when using `mkdocs serve` locally, whenever a file in `optimade/` is updated.
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
If an adapter dependency is not installed, a new type will be created with the same name. This is done for the sake of the docs. In order to still warn and return about the non-existent dependency, when trying to convert an OPTIMADE entry resource, the string "optimade.adapters" is searched for in the representation of the "new" type created. This is because the new type will show up as belonging to the file in which is was created. This depends on a dependency never coming from within "optimade.adapters", which makes sense, since it will always be external libraries and packages.
|
Other final comment is that default values of pydantic fields aren't documented at the moment with pytk, which isn't a big deal, except maybe for the docs of We might consider updating the config docs to make it clearer what the defaults are, though we are showing the snippet of an example config, some fields are missing here (like the new |
Ah, I forgot you also had #426 open, let's do this there instead :) |
New
invoketask to create "API Reference".This new task will go through the
optimadefolder and all subfolders, excluding data folders. A markdown file will be created for every python file and the folder tree will be kept..pagesfiles will also be created and__init__.pyfiles will be foregone, since the mkdocstrings plugin does not respect what can be imported from the root of a module and what can not, it will simply add everything with a docstring under the module ...Due to the
__init__.pyfile being foregone, the adapters that were defined in said files have been moved to anadapter.pyfile. No other__init__.pyfiles define classes and functions that need to be moved to separate files, so all good.The new task has a flag
--pre-clean, which will completely remove theapi_referencedocs sub-folder and recreate all the files anew.Otherwise the task will check the content of existing files to determine whether it needs to rewrite them or skip writing. This will of course not remove "old" files, for this the
--pre-cleanflag should be added.This task also goes for the philosophy to write the table of contents of the "API Reference" using the proper folder and file names, i.e., lower case and including
_if present.Larger overhaul
INSTALL.mdhas been renamed toinstall.mdand moved to/docs. The link to it mentioned inREADME.mdhas been changed to link to theinstall.md's file in the online documentation.All links (which created build warnings) have been fixed, either by creating symlinks in the
/docsfolder or writing the correct link name.Fancy new logo and icon.
Here, I'm just using the new OPTIMADE logo (svg), converting it to a png matching the size of the existing files.
Use black/red for prime/accent color.
Update various plugins and markdown extensions to the newest recommended editions (
codehilite->pymdown.highlight).Also added other
pymdownextensions.Doc string standard (example)
I have updated a file with the recommended format of doc strings; a very specific kind of Google doc string.
This is done like this to make
mkdocstringsas happy as possible.Example file:
entries.pyEDIT: Excluding
__json_encoder__Added filter to exclude
__json_encoder__from the documention builds.This fixes #365.
Note, the first filter added is the default filter used. It excludes all entities starting with
_, i.e., private stuff, while including everything starting with two underscores;__. This is done do include__init__, but then also finds__json_encoder__.Another way of doing this would be to first exclude all entities starting with an underscore (
_) and then add a filter including specifically__init__.Edit: I'm excluding also
__all__and__config__. Since there are other "special" methods we would like to include, like__default__and overriding__getattr__, it seems it doesn't matter anymore whether we're adding a new explicit exclude or include (concerning the discussion immediately above here).