Skip to content

BUG : fix non-uniform grids in pcolorfast #4228

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 1 commit into from
Apr 19, 2015
Merged

BUG : fix non-uniform grids in pcolorfast #4228

merged 1 commit into from
Apr 19, 2015

Conversation

tacaswell
Copy link
Member

Closes #4227

attn @mdboom @efiring This fixes the problem, but I have no idea why (I just looked at the c++ header, it wanted ints so I made sure it get ints).

@tacaswell tacaswell mentioned this pull request Mar 16, 2015
Closed
@tacaswell tacaswell added this to the next point release milestone Mar 16, 2015
@tacaswell
Copy link
Member Author

We might also want to back-port this to color-overhaul.

efiring
efiring reviewed Mar 16, 2015
View reviewed changes
lib/matplotlib/image.py
width = width * magnification
height = height * magnification
width = int(width * magnification)
height = int(height * magnification)
Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me. The only question it raises in my mind is whether after multiplication by magnification, a rounding operation would be better than truncation.

@efiring
Copy link
Member

efiring commented Mar 16, 2015

There is no point in backporting because the CXX version was handling the conversion to int. That also tells me there is no need to add a rounding step; what you have is fine.
Another way to fix the problem would be to put the conversion to int back in the C++; that makes the _pcolor2 function friendlier, but given that it is private, I have no objection to letting it be fussy about inputs. It's simpler.

@tacaswell tacaswell force-pushed the pcolorfast_nonuniform_fix branch from be0aed4 to 613ea28 Compare April 19, 2015 17:09
@tacaswell
Copy link
Member Author

@efiring rebased to fix conflict in tests.

Re-reading this I read your suggestion about rounding, implemented it and then re-read your comment about not implementing it.

I think the rounding is ok as this is determining how many pixels in the agg layer that the image will take up. Assuming travis passes, I think we should leave it as-is.

@tacaswell
Copy link
Member Author

well, of course travis is going to pass, we have no test coverage on this...

@efiring
Copy link
Member

efiring commented Apr 19, 2015

Needs a rebase? Otherwise, it looks fine.

@tacaswell tacaswell force-pushed the pcolorfast_nonuniform_fix branch from 613ea28 to 8e64eed Compare April 19, 2015 19:08
@tacaswell
Copy link
Member Author

@efiring rebased.

efiring added a commit that referenced this pull request Apr 19, 2015
@efiring
Merge pull request #4228 from tacaswell/pcolorfast_nonuniform_fix
be216b2 
BUG : fix non-uniform grids in pcolorfast
@efiring efiring merged commit be216b2 into matplotlib:master Apr 19, 2015
@tacaswell tacaswell deleted the pcolorfast_nonuniform_fix branch May 16, 2015 03:25
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.0
Development

Successfully merging this pull request may close these issues.

pcolorfast fails in master when image is not uniform
2 participants
@tacaswell @efiring