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

BUG : fix non-uniform grids in pcolorfast #4228

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

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
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
Copy link
Member Author

@efiring rebased.

efiring added a commit that referenced this pull request Apr 19, 2015
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pcolorfast fails in master when image is not uniform
2 participants