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 of docs, a few minor changes #18973

Merged
merged 4 commits into from Dec 29, 2017
Merged

Spellcheck of docs, a few minor changes #18973

merged 4 commits into from Dec 29, 2017

Conversation

tommyod
Copy link
Contributor

@tommyod tommyod commented Dec 28, 2017

This PR continues my read-through of the docs, the previous PRs submitted are #18941 and #18948.

The following edits have been made:

  • Missing periods and colons added before introducing code examples.
  • Increased number of function references (clickable links).
  • Cleared up a few sentences which I found unclear.

Feedback is welcome.

Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

Only partway through, looks good so far.

Style question: we have quite a few places like

The :meth:`Series.cov` method does...

I prefer just saying

:meth:`Series.cov` does...

I think it's clear from the context that we're talking about a method.

@@ -315,7 +316,7 @@ Basic multi-index slicing using slices, lists, and labels.

dfmi.loc[(slice('A1','A3'), slice(None), ['C1', 'C3']), :]

You can use a ``pd.IndexSlice`` to have a more natural syntax using ``:`` rather than using ``slice(None)``
You can use ``pd.IndexSlice`` to facilitate a more natural syntax using ``:``, rather than using ``slice(None)``.
Copy link
Contributor

Choose a reason for hiding this comment

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

You could link to IndexSlice here

:class:`IndexSlice`

I think (maybe pandas.IndexSlice).


Slicing is ALWAYS on the values of the index, for ``[],ix,loc`` and ALWAYS positional with ``iloc``
Slicing is **always** on the values of the index when using ``[],ix,loc``, and
Copy link
Contributor

Choose a reason for hiding this comment

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

Except for booleans :)

Perhaps we can rephrase this as "primarily on the values" and mention elsewhere (perhaps after iloc) that booleans are also allowed.

@@ -47,17 +48,18 @@ NA/null values *before* computing the percent change).
Covariance
~~~~~~~~~~

The ``Series`` object has a method ``cov`` to compute covariance between series
(excluding NA/null values).
The ``Series`` object has a method :meth:`~Series.cov` to compute covariance
Copy link
Contributor

Choose a reason for hiding this comment

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

How about

:meth:`Series.cov` can be used to compute covariance between series (excluding missing values)


.. ipython:: python

s1 = pd.Series(np.random.randn(1000))
s2 = pd.Series(np.random.randn(1000))
s1.cov(s2)

Analogously, ``DataFrame`` has a method ``cov`` to compute pairwise covariances
among the series in the DataFrame, also excluding NA/null values.
Analogously, ``DataFrame`` has a method :meth:`~DataFrame.cov` to compute
Copy link
Contributor

Choose a reason for hiding this comment

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

And

Analogously, :meth:`DataFrame.cov` can be used to compute pairwise...

@codecov
Copy link

codecov bot commented Dec 28, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@10edfd0). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #18973   +/-   ##
=========================================
  Coverage          ?   91.58%           
=========================================
  Files             ?      150           
  Lines             ?    48972           
  Branches          ?        0           
=========================================
  Hits              ?    44851           
  Misses            ?     4121           
  Partials          ?        0
Flag Coverage Δ
#multiple 89.94% <ø> (?)
#single 41.72% <ø> (?)

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 10edfd0...9133861. Read the comment docs.

@jreback jreback added the Docs label Dec 28, 2017
@tommyod
Copy link
Contributor Author

tommyod commented Dec 28, 2017

Thanks for the feedback @TomAugspurger. I have submitted a new PR addressing your comments, which were helpful.

  • I agree that "... to compute it, use my_method and ..." is more succinct and clear than "... to compute it, use the method my_method and ...".
  • I am uncertain about starting a sentence with "The my_method method is..." vs. "my_method is...". The first phrasing seems to occur quite a bit in the documentation.

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.

lgtm! just some small changes. ping on green.

@@ -110,6 +114,11 @@ Several methods for computing correlations are provided:
.. \rho = \cov(x, y) / \sigma_x \sigma_y

All of these are currently computed using pairwise complete observations.
Wikipedia has articles covering the
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 these a bulleted list?

true wherever the Series elements exist in the passed list. This allows you to
select rows where one or more columns have values you want:
Consider the :meth:`~Series.isin` method of Series, which returns a boolean
vector that is true wherever the Series elements exist in the passed list.
Copy link
Contributor

Choose a reason for hiding this comment

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

generally like to use double-backticks around Series, DataFrame, Index where possible.

@@ -1123,8 +1125,6 @@ as condition and ``other`` argument.
'C': [7, 8, 9]})
df3.where(lambda x: x > 4, lambda x: x + 10)

**mask**
Copy link
Contributor

Choose a reason for hiding this comment

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

could make mask a sub-section (of the where section).

@@ -1123,8 +1125,6 @@ as condition and ``other`` argument.
'C': [7, 8, 9]})
df3.where(lambda x: x > 4, lambda x: x + 10)

**mask**

``mask`` is the inverse boolean operation of ``where``.
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the experimental from the Query section.

@@ -37,7 +37,7 @@ namespace:
- :func:`~pandas.option_context` - execute a codeblock with a set of options
that revert to prior settings after execution.

**Note:** developers can check out pandas/core/config.py for more info.
**Note:** Developers can check out ``pandas/core/config.py`` for more info.
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -79,15 +79,15 @@ Getting and Setting Options
---------------------------

As described above, ``get_option()`` and ``set_option()`` are available from the
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use :func:`get_option` and so on

@@ -99,7 +99,7 @@ Elements in the split lists can be accessed using ``get`` or ``[]`` notation:
s2.str.split('_').str.get(1)
s2.str.split('_').str[1]

Easy to expand this to return a DataFrame using ``expand``.
It's easy to expand this to return a DataFrame using ``expand``.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is

@tommyod
Copy link
Contributor Author

tommyod commented Dec 29, 2017

@jreback I've addressed your comments. I've been noting some stylistic preferences (which I agree with), such as "It is ..." instead of "It's" and using double-backticks on objects. For now I have not addressed these globally in the docs, but I hope to do so in a future PR.

@jreback
Copy link
Contributor

jreback commented Dec 29, 2017

@tommyod totally cool. updating things like this is generally, do a few docs / files at once. it becomes too hard to review otherwise.

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.


``mask`` is the inverse boolean operation of ``where``.
Mask
~~~~~~~~~
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be the same length as the title (otherwise sphinx warns)

@@ -1134,7 +1138,7 @@ as condition and ``other`` argument.

.. _indexing.query:

The :meth:`~pandas.DataFrame.query` Method (Experimental)
The :meth:`~pandas.DataFrame.query` Method
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

indexed DataFrame:
DataFrame has a :meth:`~DataFrame.set_index` method which takes a column name
(for a regular ``Index``) or a list of column names (for a ``MultiIndex``),
to create a new, indexed DataFrame:
Copy link
Contributor

Choose a reason for hiding this comment

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

capitalize as beginning of sentence?

As a convenience, there is a new function on DataFrame called
:meth:`~DataFrame.reset_index` which transfers the index values into the
DataFrame's columns and sets a simple integer index.
This is the inverse operation to ``set_index``.
Copy link
Contributor

Choose a reason for hiding this comment

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

put a :func:`DataFrame.set_index`

@@ -1728,7 +1733,7 @@ discards the index, instead of putting index values in the DataFrame's columns.

.. note::

The ``reset_index`` method used to be called ``delevel`` which is now
The ``reset_index`` method used to be called ``delevel``, which is now
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove this, delevel is removed

@tommyod
Copy link
Contributor Author

tommyod commented Dec 29, 2017

@jreback 👍 . Addressed comments.

@jreback jreback added this to the 0.23.0 milestone Dec 29, 2017
@jreback jreback merged commit 8433562 into pandas-dev:master Dec 29, 2017
@jreback
Copy link
Contributor

jreback commented Dec 29, 2017

thanks @tommyod

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.

None yet

3 participants