Fix #5693: Implemented is_sorted in C #5719

Merged
merged 2 commits into from Dec 23, 2015

Conversation

Projects
None yet
4 participants
Owner

mdboom commented Dec 22, 2015

No description provided.

@mdboom mdboom Fix #5693: Implemented is_sorted in C
d0834e3

mdboom added the needs_review label Dec 22, 2015

Owner

efiring commented Dec 22, 2015

Nice!

Owner

tacaswell commented Dec 22, 2015

tad worried what this will do with large int arrays or pd.Series but so long an np will do the corrosion it should be no worse than it is now (I think).

Owner

mdboom commented Dec 22, 2015

tad worried what this will do with large int arrays or pd.Series but so long an np will do the corrosion it should be no worse than it is now (I think).

Yeah. I think a conversion was already happening in the nanmin call (since nans don't even make sense with ints), so this is no worse. That said, there's probably some low-hanging fruit to do the easy thing if it's an int array to begin with.

@mdboom mdboom Handle more types directly
5c4f237
Owner

mdboom commented Dec 22, 2015

I've updated this to work on ints directly as well.

@tacaswell tacaswell added a commit that referenced this pull request Dec 23, 2015

@tacaswell tacaswell Merge pull request #5719 from mdboom/sorting-check
Fix #5693: Implemented is_sorted in C
aab6cfd

@tacaswell tacaswell merged commit aab6cfd into matplotlib:master Dec 23, 2015

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.004%) to 68.323%
Details

tacaswell removed the needs_review label Dec 23, 2015

Owner

tacaswell commented Dec 23, 2015

backport on this had conflicts, making sure it works locally before pushing

@tacaswell tacaswell added a commit that referenced this pull request Dec 23, 2015

@tacaswell tacaswell Merge pull request #5719 from mdboom/sorting-check
Fix #5693: Implemented is_sorted in C

Conflicts:
	lib/matplotlib/lines.py
	   Imports at the top picked up the new markers which are
	   not present on this branch (1.5.x).

	lib/matplotlib/tests/test_lines.py
	   minor clashes in _is_sorted test, resolved in favor of
	   current branch to ease future merge conflicts
db80928
Owner

tacaswell commented Dec 23, 2015

backported as db80928

Member

QuLogic commented Dec 23, 2015

I know this is merged already, but I'm curious as to the expected behaviour with respect to infinities? As you may guess from the name, std::isfinite returns false for infinity, so the behaviour is different, and there are no tests that exercise that case to show what's expected.

>>> import numpy as np
>>> x = np.arange(10.)
>>> x[4] = np.inf
>>> x
array([  0.,   1.,   2.,   3.,  inf,   5.,   6.,   7.,   8.,   9.])
>>> np.nanmin(x[1:] - x[:-1]) >= 0
False
>>> from matplotlib import _path
>>> _path.is_sorted(x)
True

Is it just not possible for there to be infinities at this location?

Owner

tacaswell commented Dec 24, 2015

That is a good point that I think we missed on review.

On Wed, Dec 23, 2015 at 6:44 PM Elliott Sales de Andrade <
notifications@github.com> wrote:

I know this is merged already, but I'm curious as to the expected
behaviour with respect to infinities? As you may guess from the name,
std::isfinite returns false for infinity, so the behaviour is different,
and there are no tests that exercise that case to show what's expected.

import numpy as np>>> x = np.arange(10.)>>> x[4] = np.inf>>> x
array([ 0., 1., 2., 3., inf, 5., 6., 7., 8., 9.])>>> np.nanmin(x[1:] - x[:-1]) >= 0False>>> from matplotlib import _path>>> _path.is_sorted(x)True

Is it just not possible for there to be infinities at this location?


Reply to this email directly or view it on GitHub
#5719 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment