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

Agg restore_region is broken #4913

Merged
merged 1 commit into from Aug 14, 2015
Merged

Agg restore_region is broken #4913

merged 1 commit into from Aug 14, 2015

Conversation

jrevans
Copy link

@jrevans jrevans commented Aug 12, 2015

The original version used to have float parameters that would be converted to integers in the agg renderer C++ file. The current version now performs some type-checking in the Python->C++ layer that will throw an exception tot he float values that are passed in. This fix makes sure that we are indeed passing in integers to the C++ method.

This addresses an issue in #4897.

# The incoming data is float, but the _renderer type-checking wants
# to see integers.
self._renderer.restore_region(region, int(x1), int(y1),
int(x2), int(y2), int(ox), int(oy))
Copy link
Member

Choose a reason for hiding this comment

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

To be pedantic, not all of the incoming data is coming in as floats. For example, region.get_extents() will return integers, which is probably why we didn't notice this bug as I think that is the typical code-path. I haven't checked what bbox.extents would return, but certainly input-sanitation should be done anyway due to the possibility of the user passing in a tuple of floats as a bbox.

How did you trigger the error?

@tacaswell tacaswell added this to the next point release milestone Aug 12, 2015
@tacaswell
Copy link
Member

Seems fine, I am also curious how you triggered this.

I assume this is fall out from the change of using CXX for wrapping agg to doing it directly with the c-api.

tacaswell added a commit that referenced this pull request Aug 14, 2015
FIX: cast to int in input to Agg restore_region

Can get floats as input, cast to int
@tacaswell tacaswell merged commit 7c3f08e into matplotlib:master Aug 14, 2015
@tacaswell
Copy link
Member

Merging this as it restores previous behavior of implicit cast to int. If we want to intentionally break that we should have that discussion later.

@jrevans jrevans deleted the issue12 branch August 17, 2015 17:17
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.

None yet

3 participants