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

DOC: explain the mode.chained_assignment option #18635

Merged
merged 2 commits into from
Dec 7, 2017

Conversation

sietse
Copy link
Contributor

@sietse sietse commented Dec 4, 2017

  • closes #xxxx (n/a)
  • tests added / passed (yes, Sphinx compiles without errors)
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry (n/a)

Make the explanation of the mode.chained_assignment option link to the explanation of chained indexing and assignment. Explain what the three possible settings do.


You can control the action of a chained assignment via the option ``mode.chained_assignment``,
which can take the values ``['raise','warn',None]``, where showing a warning is the default.
When using chained indexing, the order may determine whether a copy is returned
Copy link
Contributor

Choose a reason for hiding this comment

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

say 'order of evaluation'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that merely 'the order' is not explicit enough. Now that I am looking closer, though, I would say it's not the order of evaluation, but the order in which the user chains the indexing operations (which in turn determines the order the operations are evaluated).

I have turned the sentence into this, which I hope is also in the spirit of your edit:

When you use chained indexing, the order and type of the indexing operation
partially determine whether the result is a slice into the original object, or
a copy of the slice.

What do you think of that? I don't mean to make this a bikeshedding point, so I will implement whatever you suggest as the final version.


Pandas has the ``SettingWithCopyWarning`` because assigning to a copy of a
slice is frequently not intentional, but a mistake caused by chained indexing
returning a copy where a slice was expected. If you would like pandas to be
Copy link
Contributor

Choose a reason for hiding this comment

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

paragraph break after first sentence

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.

slice is frequently not intentional, but a mistake caused by chained indexing
returning a copy where a slice was expected. If you would like pandas to be
more or less trusting about assignment to a chained indexing expression, you
can set the :ref:`option <options>` ``mode.chained_assignment``, which can take
Copy link
Contributor

Choose a reason for hiding this comment

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

use a bulleted list for these options

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.

@jreback jreback added the Docs label Dec 5, 2017
@codecov
Copy link

codecov bot commented Dec 5, 2017

Codecov Report

Merging #18635 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18635      +/-   ##
==========================================
- Coverage   91.59%   91.57%   -0.02%     
==========================================
  Files         155      155              
  Lines       51255    51255              
==========================================
- Hits        46948    46939       -9     
- Misses       4307     4316       +9
Flag Coverage Δ
#multiple 89.44% <ø> (ø) ⬆️
#single 40.67% <ø> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.81% <0%> (-0.1%) ⬇️

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 a764663...3f22934. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 5, 2017

Codecov Report

Merging #18635 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18635      +/-   ##
==========================================
- Coverage   91.59%   91.54%   -0.05%     
==========================================
  Files         153      153              
  Lines       51257    51210      -47     
==========================================
- Hits        46949    46882      -67     
- Misses       4308     4328      +20
Flag Coverage Δ
#multiple 89.41% <ø> (-0.04%) ⬇️
#single 40.67% <ø> (-0.12%) ⬇️
Impacted Files Coverage Δ
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/plotting/_converter.py 63.44% <0%> (-3.08%) ⬇️
pandas/core/config_init.py 98.34% <0%> (-0.12%) ⬇️
pandas/core/indexes/datetimes.py 95.58% <0%> (-0.11%) ⬇️
pandas/core/frame.py 97.81% <0%> (-0.1%) ⬇️
pandas/plotting/_core.py 82.49% <0%> (+0.12%) ⬆️

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 24b8f1e...853c87f. Read the comment docs.

@sietse sietse force-pushed the chained_assignment branch from 3f22934 to 054c048 Compare December 5, 2017 21:42
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. minor change. ping on green.

which can take the values ``['raise','warn',None]``, where showing a warning is the default.
* ``'raise'`` means pandas will raise a ``SettingWithCopyException`` exception
you have to deal with.
* ``'warn'``, the default, means a ``SettingWithCopyWarning`` is printed.
Copy link
Contributor

Choose a reason for hiding this comment

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

put the default first

@jreback jreback added this to the 0.21.1 milestone Dec 6, 2017
@sietse sietse force-pushed the chained_assignment branch from 054c048 to dddb85e Compare December 6, 2017 18:18
@sietse
Copy link
Contributor Author

sietse commented Dec 6, 2017

@jreback I've put the default first, and the checks have passed. Does 'ping on green' mean to send you a message when this is done? If yes, this is your ping.

I forgot to say this last time I replied, so I'll say it now: thank you very much! Not just for the review, but for all the work you do for the pandas project. It is very much appreciated.

@jorisvandenbossche jorisvandenbossche merged commit 9629fef into pandas-dev:master Dec 7, 2017
@jorisvandenbossche
Copy link
Member

@sietse Thanks a lot! (I fixed the conflict, due to another PR that was just merged)

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.

4 participants