Updated custom_projection_example.py to work with v1.2 and newer #1579

Merged
merged 2 commits into from Dec 11, 2012

3 participants

@joferkington

custom_projection_example.py is currently broken with matplotlib v1.2.x.

Compare the custom_projection_example.py run with matplotlib 1.1 Example with 1.1

with the custom_projection_example.py run with matplotlib 1.2 Example with 1.2

@joferkington

I should have added this information here instead of on the mailing list. At any rate, here's the rationale for the changes, beyond simply fixing the example:

At some point transforms.Transform was slightly refactored. (Particularly,
this commit:
8bbe2e5 )

It's certainly makes things more logical and consistent, but it changed what methods need to be overridden when subclassing
Transform.

It looks like Phil caught the changes for the polar and geo projections, but the custom projection example was never updated. This led to the example failing for newer (1.2.0 and master) versions.

However, it's worth elaborating on the changes to the Transform class in the comments in the example.

With versions 1.1.x and older, one had to directly implement a
transform method when subclassing transforms.Transform,
otherwise a NotImplemented error would be raised.

With versions 1.2.x and newer, the preferred way appears to be to implement
things in a separate transform_affine or transform_non_affine method and
not explicitly implement a transform method.

If you implement the non-affine portion directly in the transform method
without overriding transform_non_affine, it leads to strange drawing bugs
with v1.2 that did not occur with older versions. (And thus the example being broken.)

On the other hand, for compatibility with versions 1.1 and older, you have
to explicitly implement the transform method as well, otherwise you'll get
the NotImplemented error.

Therefore, now one needs to explicitly implement both the
transform_non_affine and transform methods of a custom non-affine transform
for compatibility with 1.1 and older as well as 1.2 and newer.

Similarly, one needs to implement both the transform_path_non_affine and the
transform_path methods for compatibility with newer and older versions of
matplotlib.

@pelson pelson and 1 other commented on an outdated diff Dec 10, 2012
examples/api/custom_projection_example.py
ipath = path.interpolated(path._interpolation_steps)
return Path(self.transform(ipath.vertices), ipath.codes)
+ transform_path_non_affine.__doc__ = \
+ Transform.transform_path_non_affine.__doc__
+
+ # Once again, for compatibility with matplotlib v1.1 and older, we need
+ # to explicitly override ``transform_path``. With v1.2 and newer, only
+ # overriding the ``transform_path_non_affine`` method is sufficient.
+ transform_path = transform_path_non_affine
@pelson
Matplotlib Developers member
pelson added a line comment Dec 10, 2012

We could do a version specific override here. Something like:

if matplotlib.__version__ < '1.2':
    transform_path = ...
    ...
@joferkington
joferkington added a line comment Dec 11, 2012

Good point! Also a version-specific conditional would potentially be more future-proof, as well as "stretching the legs" of the new functionality. I'll add that here directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pelson
Matplotlib Developers member

Thanks for doing this - much appreciated! I have commented that we might consider applying the compatibility code only if the version requires it, that way we are stretching the legs of the new functionality for newer versions, whereas we can still run the code for older. What do you think?

@pelson
Matplotlib Developers member

Other than that 👍

@pelson
Matplotlib Developers member

P.S. Once we are happy with this, we will want to apply it to the v1.2.x branch.

@mdboom
Matplotlib Developers member

Thanks for catching this. +1 (and I agree with all of @pelson's comments).

@joferkington

I agree, the version conditional makes the intent more clear, in case someone doesn't read the comments closely. Also, it's arguably more future-proof than re-implementing the default transform method. Done in joferkington@2a73c12. Thanks!

@pelson pelson merged commit c13f629 into matplotlib:master Dec 11, 2012

1 check passed

Details default The Travis build passed
@pelson
Matplotlib Developers member

Thanks @joferkington - keep up the great work!

@joferkington

Thanks!

@mdboom
Matplotlib Developers member

Cherry-picked to v1.2.x and will update the online docs shortly.

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