8000 Fix #5693: Implemented is_sorted in C by mdboom · Pull Request #5719 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Fix #5693: Implemented is_sorted in C #5719

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 23, 2015
Merged

Fix #5693: Implemented is_sorted in C #5719

merged 2 commits into from
Dec 23, 2015

Conversation

mdboom
Copy link
Member
@mdboom mdboom commented Dec 22, 2015

No description provided.

@mdboom mdboom added this to the Critical bugfix release (1.5.1) milestone Dec 22, 2015
@efiring
Copy link
Member
efiring commented Dec 22, 2015

Nice!

@tacaswell
Copy link
Member

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).

@mdboom
Copy link
Member Author
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
Copy link
Member Author
mdboom commented Dec 22, 2015

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

tacaswell added a commit that referenced this pull request Dec 23, 2015
@tacaswell
Merge pull request #5719 from mdboom/sorting-check
aab6cfd 
Fix #5693: Implemented is_sorted in C
@tacaswell tacaswell merged commit aab6cfd into matplotlib:master Dec 23, 2015
@tacaswell
Copy link
Member

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

tacaswell added a commit that referenced this pull request Dec 23, 2015
@tacaswell
Merge pull request #5719 from mdboom/sorting-check
db80928 
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 mer
8000
ge conflicts
@tacaswell
Copy link
Member

backported as db80928

@tacaswell tacaswell mentioned this pull request Dec 23, 2015
Closed
@QuLogic
Copy link
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?

@tacaswell
Copy link
Member

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)
.

@mdboom mdboom mentioned this pull request Dec 29, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.5.1
Development

Successfully merging this pull request may close these issues.
4 participants
@mdboom @efiring @tacaswell @QuLogic
0