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

PERF: RangeIndex.is_monotonic_inc/dec #13749

Closed
wants to merge 1 commit into from

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Jul 22, 2016

  • tests added / passed
  • passes git diff upstream/master | flake8 --diff
  • whatsnew entry

as the same as is_unique, it should be monotonic by definition

%timeit -n 1 -r 1 pd.RangeIndex(0, 1000000).is_monotonic_increasing

# current master
#1 loop, best of 1: 17.2 ms per loop

# this PR
#1 loop, best of 1: 45.1 µs per loop

@sinhrks sinhrks added the Performance Memory or execution speed performance label Jul 22, 2016
@sinhrks sinhrks added this to the 0.19.0 milestone Jul 22, 2016
@codecov-io
Copy link

codecov-io commented Jul 22, 2016

Current coverage is 84.57% (diff: 100%)

No coverage report found for master at b60e42b.

Powered by Codecov. Last update b60e42b...8e25563

self.assertTrue(index.is_monotonic)
self.assertTrue(index.is_monotonic_increasing)
self.assertTrue(index.is_monotonic_decreasing)

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a test for the empty case as well?

In [23]: pd.RangeIndex().is_monotonic_increasing
Out[23]: True

In [24]: pd.RangeIndex().is_monotonic_decreasing
Out[24]: True

Copy link
Member

Choose a reason for hiding this comment

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

Although this should maybe rather raise an error (@jreback)

Other index types to raise without arguments, and range() does as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jorisvandenbossche empty case is coverd by (1, 1).

pd.RangeIndex(1, 1).values
# array([], dtype=int64)

Let me issue a separate PR to prohibit empty construction.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, assumed that RangeIndex() does not has a step yet, but that is of course wrong :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I don't think RangeIndex() should be permitted, though I seem to remember needing it for unpickling or somesuch.......

Copy link
Contributor

Choose a reason for hiding this comment

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

let's make a new issue about the empty RangeIndex()

@jreback jreback closed this in 3fdcea6 Jul 25, 2016
@sinhrks sinhrks deleted the perf_range branch January 8, 2017 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance Memory or execution speed performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants