Skip to content

TST: Allow for multiple variables on the same line in docstring validation #28811

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

Conversation

ChiefMilesEdgeworth
Copy link
Contributor

Allow for multiple variables to share the same type and description in documentation. For example, i, j, k : int Would be three variables, i, j, and k, which are all ints.

  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry - I'm not sure how to add this, if it needs done could I get a pointer?

Allow for multiple variables to share the same type and description in documentation. For example, `i, j, k : int` Would be three variables, i, j, and k, which are all ints.
This was generating errors because it got fixed in master after I made the branch. I'm sure this will be a merge conflict, and I can work it out there first.
@ChiefMilesEdgeworth ChiefMilesEdgeworth marked this pull request as ready for review October 7, 2019 13:11
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

Can you add test(s) for this change? There is a test directory next to the validation script you can put those in

Added a test for if the spacing is off on a multivariable oneline parameter descriptoin
@ChiefMilesEdgeworth
Copy link
Contributor Author

Can you add test(s) for this change? There is a test directory next to the validation script you can put those in

I put in a test for if spacing is off. Other than that, it's a little hard to test because I can't have a test with more than one wrong parameter. The validation script uses sets, so the order can change between runs.

@WillAyd
Copy link
Member

WillAyd commented Oct 8, 2019

Can you not put a test in class GoodDocStrings that has this behavior to make sure it works?

@ChiefMilesEdgeworth
Copy link
Contributor Author

Oh, yeah. I'll do that real quick. Sorry, didn't cross my mind.

Adds a test for if multiple variables are typed and described on the same line in a docstring.
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

minor comment otherwise lgtm @datapythonista if you have any thouhts

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

nice PR, just couple of typos, but this is very useful

Removed the functionality of tests, since it isn't important for documentation. Fixed a couple of small typos.
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

OK by me - merge whenever happy @datapythonista

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Looks good to me as well, thanks a lot for the contribution!

@WillAyd WillAyd added this to the 1.0 milestone Oct 11, 2019
@WillAyd WillAyd merged commit 54b1151 into pandas-dev:master Oct 11, 2019
@WillAyd
Copy link
Member

WillAyd commented Oct 11, 2019

Thanks @Nabel0721 !

@WillAyd WillAyd mentioned this pull request Oct 11, 2019
5 tasks
@ChiefMilesEdgeworth ChiefMilesEdgeworth deleted the Documenation_oneline_multivariable_description branch October 12, 2019 00:05
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
bongolegend pushed a commit to bongolegend/pandas that referenced this pull request Jan 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants