Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A version of automodapi for PlasmaPy #1105

Merged
merged 207 commits into from
May 13, 2021

Conversation

rocco8773
Copy link
Member

@rocco8773 rocco8773 commented Apr 10, 2021

This PR creates the sphinx extension package plasmapy_sphinx. This package defines two reST directives automodapi and automodsumm, which are an adaption of the likely named directives from sphinx_automdoapi. There are a few significant distinctions of our directives over the sphinx_automodapi directives that made me add the code here and not push it upstream to sphinx_automodapi.

  1. I introduced the concept of groups for the directives. So, when an module is inspected for its objects, the found objects are categorized into group types (modules, functions, classes, exceptions, etc.). As a result, the sphinx_automodapi options like :functions-only: are replaced by :groups: functions.

  2. This move to the groups options allows for multiple groups to be displayed at once for automodsumm by doing

    .. automodsumm:: foo.bar
       :groups: functions, classes
    
    .. automodapi:: foo.bar
       :groups: functions, classes

    It additionally allows for custom groups to be defined.

  3. Configuration values (that are defined in conf.py) are introduced that allows you define custom groups. To define a custom group the automodapi_custom_groups configuration value needs to be defined like

    automodapi_custom_groups = {
        "mygroup" : {
            "title": "My Custum Group",
            "dunder": "__mygroup__",
        },
    }

    In this example mygroup would be the name of the custom group, "title" would be the title rendered above the autosummary table when automodapi is used, and "dunder" is the name of the dunder variable used to list the custom group objects. This __mygroup__ would be placed in the module being documented with the custom group and is defined just like __all__.

  4. The automodapi directive is an official autodoc directive and not a function connected to the source-read event. This allows automodapi to benefit from all the default functionality associated with the autodoc directives.

@rocco8773 rocco8773 self-assigned this Apr 10, 2021
@github-actions github-actions bot added docs PlasmaPy Docs at http://docs.plasmapy.org plasmapy.formulary Related to the plasmapy.formulary subpackage labels Apr 10, 2021
Copy link
Member

@StanczakDominik StanczakDominik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mobile app is not letting me do a single line comment for some reason.

@@ -0,0 +1,18 @@
# The code here was adapted from v0.14.0.dev0 of sphinx_automodapi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should note the license of automodapi here! Also, dev0 is probably autogenerated and it'd be better to point to a particular commit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still a lot of clean up to be had here and there, including properly linking to sphinx_automodapi. I just want to get the bulk working and then I'll go back to dot the i's and cross the t's.

@rocco8773
Copy link
Member Author

@StanczakDominik I'm not exactly sure why the tests are failing. I believe the runner currently thinks it has to run tests on any .py file in docs/ and sphinx isn't included in the dependencies for those test environments, so it fails. I tried changing a few things in the setup.cfg, but with no success. Do you mind looking into this? I'm going to be out of town through the weekend so there's no rush. Thanks.

@StanczakDominik
Copy link
Member

Sure, I'll get them passing :) don't worry about it.

Copy link
Member

@namurphy namurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A partial review focusing on docstrings in __init__.py and automodapi.py. Looks like there is some really useful functionality in there. The suggestions are pretty minor. In a few places I wasn't sure if my suggestion referred to what you intended so feel free to adjust as needed. And thank you again for doing this!

docs/plasmapy_sphinx/__init__.py Outdated Show resolved Hide resolved
docs/plasmapy_sphinx/__init__.py Outdated Show resolved Hide resolved
docs/plasmapy_sphinx/automodapi.py Outdated Show resolved Hide resolved
docs/plasmapy_sphinx/automodapi.py Outdated Show resolved Hide resolved
docs/plasmapy_sphinx/automodapi.py Outdated Show resolved Hide resolved
docs/plasmapy_sphinx/automodapi.py Outdated Show resolved Hide resolved
docs/plasmapy_sphinx/automodapi.py Show resolved Hide resolved
docs/plasmapy_sphinx/automodapi.py Outdated Show resolved Hide resolved
Comment on lines +265 to +266
`automodapi` Configuration Values
---------------------------------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of useful functionality in this section!

@github-actions github-actions bot added the plasmapy.particles Related to the plasmapy.particles subpackage label Apr 30, 2021
@rebeccaringuette
Copy link

Some notations are a bit puzzling. For example, the open bracket below seems to need a term before the comma, but none is there. What is meant by this notation?

Alfven_speed(B, density[, ion, z_mean])

@namurphy
Copy link
Member

I also noticed that there was a thing on this page:

gyroradius(B, particle, *[, Vperp, T_i])

I'm not sure if there should be a comma or a space after the *. In any case, we do think that this is a great improvement!

@rocco8773
Copy link
Member Author

@rebeccaringuette and @namurphy When a the signature of a documented function shows parameters in square brackets, then those parameters are "optional" parameters. This is the behavior of Sphinx's autodoc directives and autosummary. So, in the two examples you two mentioned...

Alfven_speed(B, density[, ion, z_mean]) --> optional parameters are ion and z_mean
gyroradius(B, particle, *[, Vperp, T_i]) --> optional parameters are Vperp and T_i

You can see this throughout documentation for various Python packages. For example, python's dir() function has an optional single argument object.

@StanczakDominik StanczakDominik mentioned this pull request May 13, 2021
Copy link
Member

@StanczakDominik StanczakDominik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I've read the docs, fixed a few small typos. LveryGTM, I'd be in favor of a swift merge :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs PlasmaPy Docs at http://docs.plasmapy.org plasmapy.formulary.quantum Related to the plasmapy.formulary.quantum module plasmapy.formulary Related to the plasmapy.formulary subpackage plasmapy.particles Related to the plasmapy.particles subpackage plasmapy.plasma Related to the plasmapy.plasma subpackage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants