Skip to content

DOC: update the DataFrame.sub docstring #20358

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 3 commits into from
Mar 16, 2018

Conversation

limitup
Copy link

@limitup limitup commented Mar 15, 2018

Checklist for the pandas documentation sprint :

  • PR title is "DOC: update the docstring"
  • The validation script passes: scripts/validate_docstrings.py <your-function-or-method>
  • The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
  • The html version looks good: python doc/make.py --single <your-function-or-method>
  • It has been proofread on language by another sprint participant

Please include the output of the validation script below between the "```" ticks:

################################################################################
################################## Validation ##################################
################################################################################

Errors found:
	Use only one blank line to separate sections or paragraphs
	Errors in parameters section
		Wrong parameters order. Actual: ('other', 'axis', 'level', 'fill_value'). Documented: ('other', 'axis', 'fill_value', 'level')
		Parameter "other" has no description
		Parameter "axis" description should finish with "."
		Parameter "fill_value" description should finish with "."
		Parameter "level" description should finish with "."
	Missing description for See Also "DataFrame.rsub" reference

Validation script returns docstring style errors.

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.

Thanks for the PR!

Can you ensure with scripts/validate_docstring.py that everything is OK?
For me this gives errors for the examples.

@@ -370,16 +370,44 @@ def _get_op_name(op, special):
e NaN 2.0
"""

_sub_example_FRAME = """
>>> a = pd.DataFrame([2, 1, 1, np.nan], index=['a', 'b', 'c', 'd'],
columns=['one'])
Copy link
Member

Choose a reason for hiding this comment

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

You need to add ... at the beginning of the line here.

The same for b below

@limitup limitup closed this Mar 15, 2018
@limitup limitup deleted the docs_dataframe-sub branch March 15, 2018 14:24
@limitup limitup restored the docs_dataframe-sub branch March 15, 2018 15:06
@limitup
Copy link
Author

limitup commented Mar 15, 2018

I accidentally closed this PR. Reopening

@limitup limitup reopened this Mar 15, 2018
@codecov
Copy link

codecov bot commented Mar 15, 2018

Codecov Report

Merging #20358 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #20358      +/-   ##
==========================================
- Coverage   91.78%   91.77%   -0.01%     
==========================================
  Files         152      152              
  Lines       49196    49176      -20     
==========================================
- Hits        45153    45133      -20     
  Misses       4043     4043
Flag Coverage Δ
#multiple 90.16% <100%> (-0.01%) ⬇️
#single 41.84% <100%> (+0.02%) ⬆️
Impacted Files Coverage Δ
pandas/core/ops.py 96.35% <100%> (ø) ⬆️
pandas/core/algorithms.py 94.17% <0%> (-0.13%) ⬇️
pandas/core/arrays/categorical.py 95.06% <0%> (-0.06%) ⬇️
pandas/core/generic.py 95.85% <0%> (ø) ⬆️
pandas/core/indexes/datetimes.py 95.64% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a044239...4141e1e. Read the comment docs.

@limitup
Copy link
Author

limitup commented Mar 15, 2018

When I added the "..." for the wrapped lines there were no more validation errors.

@jorisvandenbossche
Copy link
Member

@kurtiskerstein you seem to have included some unrelated commits. If you need any help with resolving this, let us know.

@limitup
Copy link
Author

limitup commented Mar 15, 2018

Yeah. I am going back through the git documentation to figure it out. I will let you know if I have further trouble.

@limitup limitup force-pushed the docs_dataframe-sub branch from a114473 to 82d5861 Compare March 15, 2018 16:15
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.

Git problems indeed fixed!

Added two minor comments

@@ -354,7 +354,7 @@ def _get_op_name(op, special):
d NaN
>>> b = pd.DataFrame(dict(one=[1, np.nan, 1, np.nan],
... two=[np.nan, 2, np.nan, 2]),
... index=['a', 'b', 'd', 'e'])
... index=['a', 'b', 'd', 'e'])
Copy link
Member

Choose a reason for hiding this comment

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

the index is not part of the dict, so the indentation was actually correct

_op_descriptions = {
# Arithmetic Operators
'add': {'op': '+',
'desc': 'Addition',
'reverse': 'radd',
'df_examples': _add_example_FRAME},
# Subtraction Operators
Copy link
Member

Choose a reason for hiding this comment

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

this comment is not needed, as "sub" still belongs to the "Arithmetic operators" (the comment above)

@jorisvandenbossche jorisvandenbossche merged commit 4dc05b5 into pandas-dev:master Mar 16, 2018
@jorisvandenbossche
Copy link
Member

@kurtiskerstein Thanks for the PR!

@jorisvandenbossche jorisvandenbossche added this to the 0.23.0 milestone Mar 16, 2018
@limitup limitup deleted the docs_dataframe-sub branch March 17, 2018 19:19
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.

2 participants