Navigation Menu

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

Consistently use param: type in docstrings #4284

Closed
MarcoGorelli opened this issue Dec 2, 2020 · 6 comments
Closed

Consistently use param: type in docstrings #4284

MarcoGorelli opened this issue Dec 2, 2020 · 6 comments

Comments

@MarcoGorelli
Copy link
Contributor

MarcoGorelli commented Dec 2, 2020

The docstring of sample (from pymc3.sampling) is written as param : type:

    draws : int
        The number of samples to draw. Defaults to 1000. The number of tuned samples are discarded
        by default. See ``discard_tuned_samples``.

and in the docs this shows up as

image

which, given the PyMC3 docs configurations, doesn't look great.

On the other hand, fast_sample_posterior_predictive is written as param: type:

    trace: MultiTrace, xarray.Dataset, InferenceData, or List of points (dictionary)
        Trace generated from MCMC sampling.

In the docs this shows up as

image

which, given the PyMC3 docs configurations, looks better.


Possible fixes here would be:

  • change the docs configs so that param : type looks better and then apply that consistently
  • apply param: type consistently

I'd be more inclined towards the latter

UPDATE

Please wait before making a big PR for this, we may be able to resolve the custom sphinx theme

@michaelosthege
Copy link
Member

The numpydoc style guide is very clear about the spaces:

The colon must be preceded by a space, or omitted if the type is absent.

Not sure if the bug was fixed in the meantime, @OriolAbril ?

It would be great to add a numpydoc style checker to the pre-commit...

@Sayam753
Copy link
Member

Sayam753 commented Jul 3, 2021

Not sure if the bug was fixed in the meantime, @OriolAbril ?

On v3 docs, the issue is still there. Checkout the function docstring rendering here

It would be great to add a numpydoc style checker to the pre-commit...

+1. We can use this http://www.pydocstyle.org/en/4.0.0/usage.html#usage-with-the-pre-commit-git-hooks-framework

I just ran pydocstyle check locally at v3 branch

$ pydocstyle --convention=numpy . | wc -l
    3122

So, there are > 1500 docstring warnings 😭

@OriolAbril
Copy link
Member

As @Sayam753 says, this is a combination of not following numpydoc on all the docstrings in the codebase plus a bug in the v3 theme (which is now fixed). I think we should follow numpydoc and like the lint check on it for v4 and I would therefore keep this open.

@mjhajharia
Copy link
Member

this would be an easy thing to do with docs, let’s try to find a couple of numpy docstring violations and create some smaller issues.

cc: @OriolAbril

@OriolAbril
Copy link
Member

OriolAbril commented Jan 26, 2022

this is already part of https://pymc-data-umbrella--13.org.readthedocs.build/en/13/webinars/contributing_to_documentation/docstring_tutorial.html. I think creating smaller issues for this will be more work than anything. I was planning on a single issue where we'd checkmark those following numpydoc (which I didn't realize before but is the exact opposite, we should use param : type). If you can create the issue it would be great. scikit-learn/scikit-learn#21350 is a bit similar to this

@mjhajharia mjhajharia added this to Available Issues in PyMC - Data Umbrella Sprint Feb 3, 2022
@OriolAbril
Copy link
Member

Closing this in favour of #5459 which covers more changes and is more clear.

@OriolAbril OriolAbril removed this from Available Issues in PyMC - Data Umbrella Sprint Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants