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

Strictly monotonic #16555

Merged
merged 2 commits into from
Jun 1, 2017
Merged

Conversation

TomAugspurger
Copy link
Contributor

Closes #16515

I think this can go in 0.20.2, even though part of it is a (small) enhancement.

@TomAugspurger TomAugspurger added Bug Enhancement Indexing Related to indexing on series/frames, not to indexes themselves labels May 31, 2017
@TomAugspurger TomAugspurger added this to the 0.20.2 milestone May 31, 2017
assert idx.is_monotonic_increasing
assert not idx.is_strictly_monotonic_increasing

@pytest.mark.xfail(reason="buggy MultiIndex.is_monotonic_decresaing.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed an edge case in partial string indexing where we would incorrectly flip
the endpoint on a slice, since we checked for monotonicity when we needed strict
monotonicity.

Closes pandas-dev#16515
xref dask/dask#2389
@codecov
Copy link

codecov bot commented May 31, 2017

Codecov Report

Merging #16555 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16555      +/-   ##
==========================================
+ Coverage   90.75%   90.75%   +<.01%     
==========================================
  Files         161      161              
  Lines       51074    51078       +4     
==========================================
+ Hits        46353    46357       +4     
  Misses       4721     4721
Flag Coverage Δ
#multiple 88.59% <100%> (ø) ⬆️
#single 40.16% <40%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/indexes/base.py 95.75% <100%> (ø) ⬆️
pandas/core/indexes/datetimes.py 95.33% <100%> (ø) ⬆️

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 03d44f3...dcd38f0. Read the comment docs.

@codecov
Copy link

codecov bot commented May 31, 2017

Codecov Report

Merging #16555 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16555      +/-   ##
==========================================
+ Coverage   90.75%   90.75%   +<.01%     
==========================================
  Files         161      161              
  Lines       51074    51078       +4     
==========================================
+ Hits        46353    46357       +4     
  Misses       4721     4721
Flag Coverage Δ
#multiple 88.59% <100%> (ø) ⬆️
#single 40.16% <40%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/indexes/datetimes.py 95.33% <100%> (ø) ⬆️
pandas/core/indexes/base.py 95.75% <100%> (ø) ⬆️

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 03d44f3...dcd38f0. Read the comment docs.

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented May 31, 2017

Would dask like to use those is_strictly_monotonic_increasing ? Otherwise keeping them private to not further increase number of methods is also an option.

If we keep public, would put some See also's referring to the other

@jreback
Copy link
Contributor

jreback commented May 31, 2017

lgtm. merge away.

@jreback jreback merged commit cab2b6b into pandas-dev:master Jun 1, 2017
@jreback
Copy link
Contributor

jreback commented Jun 1, 2017

thanks!

TomAugspurger added a commit to TomAugspurger/pandas that referenced this pull request Jun 1, 2017
(cherry picked from commit cab2b6b)
@jorisvandenbossche
Copy link
Member

Asking again, as you merged without responding to my comment: is it needed to add this to the public API? It there demand for this attribute?
If there is not an actual usecase (apart from the internal usage to fix the bug), I would consider making them not public

@TomAugspurger
Copy link
Contributor Author

I'm slightly in favor of having it public. In terms of cognitive load, this feels like half an additional method, rather than 2 additional methods 😄 Just since these are so similar to is_monotonic_increasing and decreasing in name and functionality.

But happy hide it if you want.

@jorisvandenbossche
Copy link
Member

In terms of cognitive load, this feels like half an additional method, rather than 2 additional methods 😄 Just since these are so similar to is_monotonic_increasing

Yes, but in tab completion they don't appear together, so I would say it is back to a full additional method in feeling :-)

Since it is very easy to do this yourself as you said above (just combine it with is_unique), I would rather add that as an example to the docs and leave them private

TomAugspurger added a commit to TomAugspurger/pandas that referenced this pull request Jun 1, 2017
@TomAugspurger
Copy link
Contributor Author

Since it is very easy to do this yourself as you said above (just combine it with is_unique), I would rather add that as an example to the docs and leave them private

Perhaps I'm slow, but it took me a day to realize that strictly monotonic = monotonic & unique 😆

PR is at #16576 Let's hide it for now, and if anyone asks for it we can make it public.

@TomAugspurger TomAugspurger deleted the strictly-monotonic branch June 1, 2017 20:49
@jreback
Copy link
Contributor

jreback commented Jun 1, 2017

@jorisvandenbossche I thought this was fine. yes its a a convience method. Sure you can do it, but why not. ok with re-hiding it as @TomAugspurger PR.

TomAugspurger added a commit to TomAugspurger/pandas that referenced this pull request Jun 2, 2017
(cherry picked from commit cab2b6b)
TomAugspurger added a commit that referenced this pull request Jun 4, 2017
(cherry picked from commit cab2b6b)
TomAugspurger added a commit that referenced this pull request Jun 4, 2017
(cherry picked from commit cab2b6b)
Kiv pushed a commit to Kiv/pandas that referenced this pull request Jun 11, 2017
stangirala pushed a commit to stangirala/pandas that referenced this pull request Jun 11, 2017
yarikoptic added a commit to neurodebian/pandas that referenced this pull request Jul 12, 2017
Version 0.20.2

* tag 'v0.20.2': (68 commits)
  RLS: v0.20.2
  DOC: Update release.rst
  DOC: Whatsnew fixups (pandas-dev#16596)
  ERRR: Raise error in usecols when column doesn't exist but length matches (pandas-dev#16460)
  BUG: convert numpy strings in index names in HDF pandas-dev#13492 (pandas-dev#16444)
  PERF: vectorize _interp_limit (pandas-dev#16592)
  DOC: whatsnew 0.20.2 edits (pandas-dev#16587)
  API: Make is_strictly_monotonic_* private (pandas-dev#16576)
  BUG: reimplement MultiIndex.remove_unused_levels (pandas-dev#16565)
  Strictly monotonic (pandas-dev#16555)
  ENH: add .ngroup() method to groupby objects (pandas-dev#14026) (pandas-dev#14026)
  fix linting
  BUG: Incorrect handling of rolling.cov with offset window (pandas-dev#16244)
  BUG: select_as_multiple doesn't respect start/stop kwargs GH16209 (pandas-dev#16317)
  return empty MultiIndex for symmetrical difference on equal MultiIndexes (pandas-dev#16486)
  BUG: Bug in .resample() and .groupby() when aggregating on integers (pandas-dev#16549)
  BUG: Fixed tput output on windows (pandas-dev#16496)
  Strictly monotonic (pandas-dev#16555)
  BUG: fixed wrong order of ordered labels in pd.cut()
  BUG: Fixed to_html ignoring index_names parameter
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Enhancement Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants