FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Plot limit with transform by pelson · Pull Request #731 · matplotlib/matplotlib · GitHub

Repository navigation

Plot limit with transform - #731

Merged
mdboom merged 11 commits into
matplotlib:masterfrom
pelson:plot_limit_with_transform
Aug 21, 2012
Merged

mdboom merged 11 commits into
matplotlib:masterfrom
pelson:plot_limit_with_transform

Conversation

pelson commented Mar 1, 2012

Copy link
Copy Markdown
Member

This pull request represents a significant chunk of work to address a simple bug:

import matplotlib.pyplot as plt
import matplotlib.transforms as mtrans

ax = plt.axes()
off_trans = mtrans.Affine2D().translate(10, 10)

plt.plot(range(11), transform=off_trans + ax.transData)

print(ax.dataLim)

The result should be [10, 10, 20, 20], but the offset transform has not been taken into account.

Since a path transformation can be costly, it made sense to use the created Line's cached transform concept.
This threw up another, quite confusing bug:

import matplotlib.projections.polar as polar
import matplotlib.transforms as mtrans
import matplotlib.path as mpath
import numpy as np

full = mtrans.Affine2D().translate(1, 0) + polar.PolarAxes.PolarTransform()

verts = np.array([[0, 0], [5, 5], [2, 0]])
p = mpath.Path(verts)

tpath = mtrans.TransformedPath(p, full)
partial_p, aff = tpath.get_transformed_path_and_affine()

print full.transform_path_affine(full.transform_path_non_affine(p))
print full.get_affine().transform_path_affine(full.transform_path_non_affine(p))

The numbers themselves aren't important, suffice to say that the former is correct.

Additionally, the need for non-affine Transform subclasses to implement transform_non_affine and also copy this definition into transform too is confusing/obfuscating e.g.:

class PolarTransform(Transform):
        def transform(self, tr):
            # ...
            # do some stuff
            # ...
            return xy
        transform.__doc__ = Transform.transform.__doc__

        transform_non_affine = transform
        transform_non_affine.__doc__ = Transform.transform_non_affine.__doc__

This latter complaint is the result of an optimisation that will see little benefit (transform stacks are typically mostly Affine, and the non-affine part is easily cached).

Therefore this pull request represents a simplification (at the cost of a couple more function calls) of the current Transform framework. Whilst it is my opinion that the Transform class heirachy remains non-optimally representative of the problem space, I have tried to be pragmatic in my changes for both backwards compatibility and size of review considerations.

This pull request is independent of the invalidation mechanism upgrade being discussed in #723, and a merge between the two should be straight forward.

The tests run exactly the same as they did before commencing this work (they weren't passing on my machine in the first place, but the RMS values have not changed at all). The run time has gone up 5 seconds up to 458 seconds (~1% slower), but this includes the new tests as a result of this pull.

Note this change subtly affects the way one should implement a Transform. If you are implementing a non affine transformation, then you should
override the transform_non_affine, rather than overriding the transform & copying the transform into transform_non_affine too. e.g.:

class PolarTransform(Transform):
        def transform_non_affine(self, tr):
            # ...
            # do some stuff
            # ...
            return xy
        transform_non_affine.__doc__ = Transform.transform_non_affine.__doc__

The documentation is still representative of this change, hence there are few documentation changes included in this pull request.

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

3 participants


Back | FazBrowse Home | New Git URL