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

Spellcheck #19017

Merged
merged 7 commits into from
Jan 3, 2018
Merged

Spellcheck #19017

merged 7 commits into from
Jan 3, 2018

Conversation

tommyod
Copy link
Contributor

@tommyod tommyod commented Dec 31, 2017

This PR is a continuation of my read-through of the docs, the earlier PRs are #18941, #18973 and #18948.

The changes included in this PR include

  • Changed spelling of "python" to "Python" for consistency, same with "numpy" to "NumPy". This change was made globally, i.e. for most documents in /docs/source/.
  • A review of groupby.rst and missing_data.rst
    • Added punctuation where missing.
    • Added more function references (i.e. :meth:...) where appropriate.
    • Rewrote a few sentences.

Feedback is very welcome.

@jreback
Copy link
Contributor

jreback commented Dec 31, 2017

Changed spelling of "python" to "Python" for consistency, same with "numpy" to "NumPy". This change was made globally, i.e. for most documents in /docs/source/.

can you add a lint rule for this (and other things that we want to check globally)
in ci/lint.sh

@jreback jreback added the Docs label Dec 31, 2017
@jreback
Copy link
Contributor

jreback commented Dec 31, 2017

also can you add a note in the whats new about documentation edits (there is a section) and include these PR#'s ?
and a similar note for #18966

@codecov
Copy link

codecov bot commented Dec 31, 2017

Codecov Report

Merging #19017 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #19017   +/-   ##
=======================================
  Coverage   91.56%   91.56%           
=======================================
  Files         150      150           
  Lines       48942    48942           
=======================================
  Hits        44816    44816           
  Misses       4126     4126
Flag Coverage Δ
#multiple 89.93% <ø> (ø) ⬆️
#single 41.75% <ø> (ø) ⬆️

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 04beec7...277c1b8. Read the comment docs.

@tommyod
Copy link
Contributor Author

tommyod commented Jan 1, 2018

@jreback I've added a reference to this and earlier PRs in whatsnew/v0.23.0.txt. I don't know how to make the linting rule, since for instance numpy should be replaced with NumPy in writte documentation, but not in the accompanying code. Not sure how to enforce this.

@jreback
Copy link
Contributor

jreback commented Jan 1, 2018

rebase and push again, fixed some hanging by Travis CI

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

small comments. can you add a rule in lint.sh?

@@ -41,7 +41,7 @@ above what the in-line examples offer.
Pandas (pd) and Numpy (np) are the only two abbreviated imported modules. The rest are kept
explicitly imported for newer users.

These examples are written for python 3.4. Minor tweaks might be necessary for earlier python
These examples are written for Python 3.4. Minor tweaks might be necessary for earlier python
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make this 3.6

Copy link
Contributor

Choose a reason for hiding this comment

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

or rather say Python 3

- A dict or Series, providing a ``label -> group name`` mapping
- A Python function, to be called on each of the axis labels.
- A list or NumPy array of the same length as the selected axis.
- A dict or Series, providing a ``label -> group name`` mapping.
Copy link
Contributor

Choose a reason for hiding this comment

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

can you double-backticks Series/DataFrame when you see

@tommyod
Copy link
Contributor Author

tommyod commented Jan 2, 2018

@jreback - I've addressed the review, rebased and pushed. I could not figure out good logic for a rule in lint.sh, will come back to this in the future if I figure it out.

@WillAyd
Copy link
Member

WillAyd commented Jan 2, 2018

@tommyod the grep command below could be a starter for you. This will find any instance of numpy but subsequently exclude it if the line contains from or import

It could miss some lines like # need to import foo from numpy but should realistically catch most issues

grep -R --include="*.py" --include="*.pyx" --include="*.rst" "numpy" $path | grep -Ev "from|import"

@jreback jreback added this to the 0.23.0 milestone Jan 3, 2018
@jreback jreback merged commit 6552718 into pandas-dev:master Jan 3, 2018
@jreback
Copy link
Contributor

jreback commented Jan 3, 2018

thanks @tommyod

yeah a rule similar to

grep -R --include="*.py" --include="*.pyx" --include="*.rst" "numpy" doc/ pandas/ | grep -Ev "from|import|_numpy_|\/numpy\/"

is close, though picking up extra things so prob have to add a bit to it

would take as a followup

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.

3 participants