Skip to content

Conversation

@topper-123
Copy link
Contributor

@topper-123 topper-123 commented Mar 2, 2018

Some minor stuff regarding .ix and .loc.

1:
The README has a link about DataFrame.ix. As ix is deprecated, the link is changed to selection-by-label.

2:
In selection-by-label a minor wall of warnings come before the reader even knows what the warning are about (because this is the section about `´loc.``...). This is very "scary" introduction.

I've moved the chained-assignment warning to after the intro, and the rest to the end of the section, so the reader first gets an understanding about loc in general and then only after that gets the warnings about the pitfalls of loc.

@topper-123 topper-123 force-pushed the minor_stuff branch 2 times, most recently from 01951a6 to 378abb9 Compare March 2, 2018 17:55
@gfyoung gfyoung added Testing pandas testing functions or related to the test suite Docs Indexing Related to indexing on series/frames, not to indexes themselves labels Mar 2, 2018
@gfyoung
Copy link
Member

gfyoung commented Mar 2, 2018

I would have preferred that you broke your in PR into two separate PR's: one for the indexing and one for your added tests. It's a little easier to review, and helps to main the atomicity of the PR's / merges.

@gfyoung gfyoung added the Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate label Mar 2, 2018
@codecov
Copy link

codecov bot commented Mar 3, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19968      +/-   ##
==========================================
- Coverage   91.71%   91.65%   -0.07%     
==========================================
  Files         150      150              
  Lines       49104    49049      -55     
==========================================
- Hits        45035    44954      -81     
- Misses       4069     4095      +26
Flag Coverage Δ
#multiple 90.03% <ø> (-0.07%) ⬇️
#single 41.79% <ø> (-0.09%) ⬇️
Impacted Files Coverage Δ
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/core/missing.py 85.9% <0%> (-5.75%) ⬇️
pandas/plotting/_timeseries.py 60.82% <0%> (-4.49%) ⬇️
pandas/plotting/_converter.py 65.22% <0%> (-1.59%) ⬇️
pandas/core/indexes/interval.py 92.44% <0%> (-0.65%) ⬇️
pandas/core/indexes/datetimes.py 95.59% <0%> (-0.07%) ⬇️
pandas/core/indexes/base.py 96.66% <0%> (ø) ⬆️
pandas/compat/pickle_compat.py 75.6% <0%> (ø) ⬆️
pandas/core/indexing.py 93.02% <0%> (ø) ⬆️
pandas/plotting/_core.py 82.23% <0%> (ø) ⬆️
... and 7 more

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 fd010de...c124a89. Read the comment docs.

@topper-123 topper-123 changed the title DOC/TST: add test for np.*prod to test_nanops.py + some doc cleanup DOC: Some doc cleanup Mar 3, 2018
@jorisvandenbossche jorisvandenbossche removed Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Testing pandas testing functions or related to the test suite labels Mar 3, 2018
.. warning::

``.loc`` is strict when you present slicers that are not compatible (or convertible) with the index type. For example
Copy link
Contributor

Choose a reason for hiding this comment

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

acutallly maybe put some of this somewhere in the slicing with labels section.

dfl.loc['20130102':'20130104']
.. warning::

Copy link
Contributor

Choose a reason for hiding this comment

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

this part should be left where it was

@topper-123 topper-123 closed this May 18, 2018
@topper-123 topper-123 deleted the minor_stuff branch May 18, 2018 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docs Indexing Related to indexing on series/frames, not to indexes themselves

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants