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

UX: move to Google style for docstrings #75

Closed
RemDelaporteMathurin opened this issue Jul 15, 2020 · 4 comments
Closed

UX: move to Google style for docstrings #75

RemDelaporteMathurin opened this issue Jul 15, 2020 · 4 comments

Comments

@RemDelaporteMathurin
Copy link

RemDelaporteMathurin commented Jul 15, 2020

Enhancement: monving from reStructured style to Google style docstrings.

  • reStructured text is convenient but is really dense and hard to type and to read

  • IDE such as VS code can generate docstrings which saves time and efforts. See here

  • Google style docstrings can be supported by Sphinx using the Napoleon extension for Sphinx

The Plasma() class would be documented like this:

class Plasma(RotateSplineShape):
    """Creates a double null tokamak plasma shape that is controlled
       by 4 shaping parameters.


    Attributes:
        points ([type], optional): [description]. Defaults to None.
        name (str, optional): [description]. Defaults to 'plasma'.
        material_tag (str, optional): [description]. Defaults to 'DT_plasma'.
        workplane (str, optional): [description]. Defaults to "XZ".
        elongation (float, optional): [description]. Defaults to 2.0.
        major_radius (int, optional): [description]. Defaults to 450.
        minor_radius (int, optional): [description]. Defaults to 150.
        triangularity (float, optional): [description]. Defaults to 0.55.
        vertical_displacement (int, optional): [description]. Defaults to 0.
        num_points (int, optional): [description]. Defaults to 50.
        configuration (str, optional): [description]. Defaults to "non-null".
        x_point_shift (float, optional): [description]. Defaults to 0.1.
        solid ([type], optional): [description]. Defaults to None.
        stp_filename (str, optional): [description]. Defaults to "plasma.stp".
        color ([type], optional): [description]. Defaults to None.
        rotation_angle (int, optional): [description]. Defaults to 360.
        azimuth_placement_angle (int, optional): [description]. Defaults to 0.
        cut ([type], optional): [description]. Defaults to None.
        hash_value ([type], optional): [description]. Defaults to None.

    Raises:
        ValueError: [description]
        ValueError: [description]
        ValueError: [description]
        ValueError: [description]
    """

instead of the current:

class Plasma(RotateSplineShape):
    """Creates a double null tokamak plasma shape that is controlled
       by 4 shaping parameters.

    :param major_radius: the major radius of the plasma (cm)
    :type major_radius: float
    :param minor_radius: the minor radius of the plasma (cm)
    :type minor_radius: float
    :param triangularity: the triangularity of the plasma
    :type triangularity: float
    :param elongation: the elongation of the plasma
    :type elongation: float
    :param vertical_displacement: the vertical_displacement of the plasma (cm)
    :type vertical_displacement: float
    :param num_points: number of points to described the shape
    :type num_points: int
    :param configuration: plasma configuration
     ("non-null", "single-null", "double-null"). Defaults to "non-null")
    :type configuration: str
    :param x_point_shift: Shift parameters for locating the X points in [0, 1].
     Default to 0.1.
    :type x_point_shift: float

    :return: a shape object that has generic functionality with 4 attributes
       (outer_equatorial_point, inner_equatorial_point, high_point, low_point)
       as tuples of 2 floats
    :rtype: paramak shape object
    """

Note: some attributes might be removed from this documentation for simpler code since they belong to the inherited class (hash_value, cut, color, solid...)

@shimwell
Copy link
Collaborator

I think we originally chose the native Sphinx style docstrings in reStructured format to avoid the use of an additional Sphinx dependancy (Napoleon).

I agree that the Google style looks nicer.

We are always keen to make improvements and take on advice.

When we started using Sphinx we just wanted something that works easily. Now that we have been using Sphinx for a while we are more comfortable in adding dependencies to the workflow so I think it is a good time to reconsider that original decision.

What I would be interested to know is can we have a mixture of Google and reStructured in the same project. If so then I imagine that we can transition gradually to the Google style

It is not the most exciting part of the project but I can see the benefits.

Readthedocs currently builds on just the develop branch so we might have to tinker with those settings as well to try this.

We use VScode to autogenerate the reStructured docstrings so that is not really an win for either option.

It also might be worth explaining why Google docstrings instead of PEP0257 or Numpy but I can see it is nicer than the reStructured

What are your thoughts @billingsley-john

@RemDelaporteMathurin
Copy link
Author

@shimwell I'm not sure if Napoleon 🇫🇷 🍷 🧀 is handled like an actual dependency though. But @billingsley-john can certifies I don't understand much about how Sphinx works 😉

@billingsley-john
Copy link

I will test whether a mixture Google and reStructured can be used in the same project. As mentioned by @shimwell, this would facilitate the gradual movement to Google-style over time

@billingsley-john
Copy link

Docs now support both Google and reStructured/Sphinx styles. Added in #99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants