John, I think we really need copy (and maybe deepcopy) functions that work with all transforms, not just Separable transforms. This looks fairly easy except for one thing: the transform creation functions return objects that don't provide any clean way of distinguishing among the types of transform. type(trans) reports <type 'Affine'>, regardless of what kind of transform it really is. I have not been able to figure out where this is coming from. One can't cleanly use hasattr(funcxy) to detect a Nonseparable transform because all transforms have the attribute, whether they use it or not. I could use "try: trans.get_funcxy()" and catch the exception, but that is ugly. (And the second time I tried it, it hung ipython.) I suspect you have thought about this already--do you have any suggested solutions? Is there at least a simple way to get type(trans) to work right? From the code it looks like it should, so there appears to be a bug in the code or in cxx. Eric
John,
I will respond to the more philosophical parts of your message later.
I have committed changes to _transforms.* and transforms.py along the
lines of your suggestions for a quick improvement to the ease of drawing
with offsets.
> The best way may be for the extension code to provide a shallowcopy
> method and require derived transform classes to implement it. All
> references will be preserved, but a new object will be created.
>
> We only need this for SeparableTransformation and
> NonseparableTransformation but the methods will also have to be
> defined virtually in the base classes.
>
> We have to think about what should be preserved in the shallow
> copies. For the use case at hand, we want to preserve the references
> to the values but not the offset transform.
>
I think I got this right--or at least as right as the existing deepcopy
methods--but it would be good if you, or another c++ wizard, could take
a look. The way I have it seems to work as intended, but testing has
been light, and I don't really know c++. I have never written any from
scratch, only modified existing code.
> I'm not so sure that deepcopy is really needed. I can't think of a
> use case off hand.
I think it is needed for pickling, but I suspect that the screwiness of
the objects that the _transforms module produces would prevent pickling
in any case. Other than that, I don't see any point in the deepcopy
methods, or the corresponding (and redundant) deepcopy functions in
transforms.py. I marked with comments a block that I think should be
excised from transforms.py.
The convenience function I came up with is transforms.offset_copy:
def offset_copy(trans, fig=None, x=0, y=0, units='inches'):
'''
Return a shallow copy of a transform with an added offset.
args:
trans is any transform
kwargs:
fig is the current figure; it can be None if units are 'dots'
x, y give the offset in units of 'inches' or 'dots'
units is 'inches' or 'dots'
'''
newtrans = trans.shallowcopy()
if units == 'dots':
newtrans.set_offset((x,y), identity_transform())
return newtrans
if not units == 'inches':
raise ValueError('units must be dots or inches')
if fig is None:
raise ValueError('For units of inches a fig kwarg is needed')
tx = Value(x) * fig.dpi
ty = Value(y) * fig.dpi
newtrans.set_offset((0,0), translation_transform(tx, ty))
return newtrans
Minimal testing and illustration of the use of offsets is now in
examples/transoffset.py. It includes cartesian and polar coordinates.
Eric
>>>>> "Eric" == Eric Firing <ef...@ha...> writes:
Eric> return newtrans if not units == 'inches': raise
Eric> ValueError('units must be dots or inches') if fig is None:
This all looks great and I like the interface. My only suggestions is
to add points (1/72. inches) since this is commonly used throughout
matplotlib, is easy, and is the most common distance metric used in
graphics.
Eric> Minimal testing and illustration of the use of offsets is
Eric> now in examples/transoffset.py. It includes cartesian and
Eric> polar coordinates.
Excellent!
JDH
>>>>> "Eric" == Eric Firing <ef...@ha...> writes: Eric> John, I think we really need copy (and maybe deepcopy) Eric> functions that work with all transforms, not just Separable Eric> transforms. This looks fairly easy except for one thing: Eric> the transform creation functions return objects that don't Eric> provide any clean way of distinguishing among the types of Eric> transform. type(trans) reports <type 'Affine'>, regardless I've seen this, I thinki it's a problem with pycxx but am not sure Eric> of what kind of transform it really is. I have not been Eric> able to figure out where this is coming from. One can't Eric> cleanly use hasattr(funcxy) to detect a Nonseparable Eric> transform because all transforms have the attribute, whether Eric> they use it or not. I could use "try: trans.get_funcxy()" Again, this is a problem with pycxx. You cannot do inheritance where B and C inherit some methods from A unless all methods are in A, B and C. It's ugly but that is the way it is for now. So I define all the methods in the base class and raise if they are not available. Unfortunately, pycxx is not actively developed so I doubt this will change . Eric> and catch the exception, but that is ugly. (And the second Eric> time I tried it, it hung ipython.) Eric> I suspect you have thought about this already--do you have Eric> any suggested solutions? Is there at least a simple way to Eric> get type(trans) to work right? From the code it looks like Eric> it should, so there appears to be a bug in the code or in Eric> cxx. The best way may be for the extension code to provide a shallowcopy method and require derived transform classes to implement it. All references will be preserved, but a new object will be created. We only need this for SeparableTransformation and NonseparableTransformation but the methods will also have to be defined virtually in the base classes. We have to think about what should be preserved in the shallow copies. For the use case at hand, we want to preserve the references to the values but not the offset transform. I'm not so sure that deepcopy is really needed. I can't think of a use case off hand. As I respond, I wonder if we are applying the right solution to the wrong problem. I think these changes are worth doing because they are easy and work with the existing code and are useful. But in the longer run, I think the offsets, while useful, can be better accomplished by developing a transform chain as Jouni suggested. Normal affine multiplication doesn't work since the transformations may be nonlinear. But we should be able to do something like (here in python but this would probably be in the extension code) class CompositeTransform: def __init__(self, transforms): self._transforms = transforms def xy_tup(self, xy): for transform in self._transforms: xy = transform.xy_tup(xy) return xy Removing the offset transforms would break internal and external code, but would probably be a cleaner solution in the long run. JDH