Skip to content

Updating docstring of a class according to PEP 257 in a file #4449

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

Merged
merged 9 commits into from
Jan 31, 2021

Conversation

ashish-hacker
Copy link
Contributor

@ashish-hacker ashish-hacker commented Jan 29, 2021

link to the concerned issue #4414

Depending on what your PR does, here are a few things you might want to address in the description:

  • what are the (breaking) changes that this PR makes?
  • This PR is about updating docstrings to PEP 257.
  • important background, or details about the implementation
  • Used PEP 257
  • are the changes—especially new features—covered by tests and docstrings?
  • Used PEP 257 for docstrings instead of numpy docstrings
  • Done style checking as mentioned in this

link to the changed docstrings

@ashish-hacker ashish-hacker changed the title Updating docstring of a class according to PEP 257 in [metropolis.py](https://github.com/pymc-devs/pymc3/blob/2d3ec8f3afcbaa9dcb8401af5fff6932ad7fd158/pymc3/step_methods/metropolis.py#L97 Updating docstring of a class according to PEP 257 in [metropolis.py](https://github.com/pymc-devs/pymc3/blob/2d3ec8f3afcbaa9dcb8401af5fff6932ad7fd158/pymc3/step_methods/metropolis.py#L97) Jan 29, 2021
@ashish-hacker ashish-hacker changed the title Updating docstring of a class according to PEP 257 in [metropolis.py](https://github.com/pymc-devs/pymc3/blob/2d3ec8f3afcbaa9dcb8401af5fff6932ad7fd158/pymc3/step_methods/metropolis.py#L97) Updating docstring of a class according to PEP 257 in a file Jan 29, 2021
@ashish-hacker
Copy link
Contributor Author

@michaelosthege Kindly review it.

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

I think the numpydoc formatting should be kept and change only where the docstring is, I don't think napoleon supports that format.

Can you generate docs locally? I amnactually not sure if there is any documentation on how to do so anywhere

@michaelosthege
Copy link
Member

Yes, the formatting of the Parameters section needs to remain the same.
And the first line in the __init__ docstring should be something like "Create an instance of a Metropolis stepper"

@michaelosthege michaelosthege marked this pull request as draft January 29, 2021 23:01
@ashish-hacker ashish-hacker marked this pull request as ready for review January 30, 2021 04:26
"""Create an instance of a Metropolis stepper

Arguments:
vars -- List of variables for sampler (default None)
Copy link
Member

Choose a reason for hiding this comment

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

This looks very different from all our other doc strings which follow the numpy style guide, can you make it adhere to that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh Sorry! My bad. I will fix it right away

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @twiecki : )

@twiecki twiecki merged commit b77e2d1 into pymc-devs:master Jan 31, 2021
@twiecki
Copy link
Member

twiecki commented Jan 31, 2021

Thanks @ashish-hacker!

@ashish-hacker
Copy link
Contributor Author

ashish-hacker commented Feb 1, 2021

Thanks @twiecki for merging. Can I do this for all the classes for all the files?

@Sayam753
Copy link
Member

Sayam753 commented Feb 1, 2021

If the entire codebase docstrings are to be moved, it will be a good idea to ensure they follow numpy style using pydocstyle and fix them alongside. See #3713

@ashish-hacker
Copy link
Contributor Author

Sure @Sayam753 :) Are you still working on #3713 ?

@Sayam753
Copy link
Member

Sayam753 commented Feb 1, 2021

No. But will be happy to contribute in these infrastructural issues along with #4354 and #4044.
Also, I am not sure if there are plans to resolve them before or after v4 release.

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

Successfully merging this pull request may close these issues.

5 participants