I think it's partly in there in a non-functional form,
> the patch I've attached removes it and adds a new
> function to the Axes class called directshow. This
> accepts the same syntax as imshow (where relevant)
> rather than adding options to imshow; I chose to do this
> as my old syntax for passing through imshow wasn't that
> easy to understand didn't makes the different
> functionality clear. This function calls a class call
> DirectImage which inherits from AxesImage.
I have mixed feelings about making this a separate class / separate
function. The current axes imshow / image.AxesImage is already
overloaded (handling MxN, MxNx3, MxNx3, _image.Image and PIL images.
What is the logic of making a special functions/classes case for MxNx4
with directshow. On the one hand, I appreciate the desire to simplify
the code by pulling it into separate classes and functions. On the
other hand, I wonder if it will confuse users to have one separate
function for UInt8 images and another for everything else. Another
worry I have about the separate DirectImage class is that it copies
much of the image resize functionality from AxesImage, including the
broken handling of aspect='preserve'. This too argues for keeping as
much functionality in AxesImage as possible, testing for A.typecode()
where appropriate. What do you think?
Also, note the matplotlib CVS has added several new image
interpolation methods. Some of these need the parameters
filternorm=1,
filterrad=4.0
as described in the imshow docstring. Since directimage exposes the
interpolation method, shouldn't it also expose these new params.
> I've also rewritten my c++ image object creation
> function called frombyte to take an unsigned byte array
> as input rather than a buffer. By using the
> std::memcopy function rather than a loop for copying the
> speed advantage of passing data in as a buffer
> disappears and using arrays is generally more intuitive.
> The function still only takes x*y*4 arrays as input at
> the moment as the processor time decrease from not using
> loops to copy is fairly significant.
Looks good to me. I'll hold off on applying these changes until I
hear from you on the issues above.
Thanks!
JDH
I'm happy for it be be called from imshow but I can't think of a syntax
to do so which isn't going to be confusing. The existing variations to
imshow are all based upon the type of the input image and this won't
work here - the case of someone who wants to plot a MxNx4 UInt8 array
and have the values it contains normalised being the problem. Another
keyword parameter could be added which only takes effect in this case; I
thought this likely to confuse but if you prefer it I'm happy.
Nick
···
On Tue, 2005-05-31 at 10:28 -0500, John Hunter wrote:
I have mixed feelings about making this a separate class / separate
function. The current axes imshow / image.AxesImage is already
overloaded (handling MxN, MxNx3, MxNx3, _image.Image and PIL images.
What is the logic of making a special functions/classes case for MxNx4
with directshow. On the one hand, I appreciate the desire to simplify
the code by pulling it into separate classes and functions. On the
other hand, I wonder if it will confuse users to have one separate
function for UInt8 images and another for everything else. Another
worry I have about the separate DirectImage class is that it copies
much of the image resize functionality from AxesImage, including the
broken handling of aspect='preserve'. This too argues for keeping as
much functionality in AxesImage as possible, testing for A.typecode()
where appropriate. What do you think?
Sorry to reply to my own message but I realised just after sending it
that it didn't make any sense (as MxNx4 images aren't normalised
anyway). I now realise that it makes sense to assume that an MxNx4
UInt8 image will contain pixel data as John suggested and I'll rewrite
my code to implement this.
It also occurred to me that it would make sense to change the PIL code
to avoid the UInt8 -> Float -> UInt transition. I'll try implementing
this after making the alterations John suggested.
Nick
···
On Wed, 2005-06-01 at 12:39 +0100, Nicholas Young wrote:
On Tue, 2005-05-31 at 10:28 -0500, John Hunter wrote:
> I have mixed feelings about making this a separate class / separate
> function. The current axes imshow / image.AxesImage is already
> overloaded (handling MxN, MxNx3, MxNx3, _image.Image and PIL images.
> What is the logic of making a special functions/classes case for MxNx4
> with directshow. On the one hand, I appreciate the desire to simplify
> the code by pulling it into separate classes and functions. On the
> other hand, I wonder if it will confuse users to have one separate
> function for UInt8 images and another for everything else. Another
> worry I have about the separate DirectImage class is that it copies
> much of the image resize functionality from AxesImage, including the
> broken handling of aspect='preserve'. This too argues for keeping as
> much functionality in AxesImage as possible, testing for A.typecode()
> where appropriate. What do you think?
I'm happy for it be be called from imshow but I can't think of a syntax
to do so which isn't going to be confusing. The existing variations to
imshow are all based upon the type of the input image and this won't
work here - the case of someone who wants to plot a MxNx4 UInt8 array
and have the values it contains normalised being the problem. Another
keyword parameter could be added which only takes effect in this case; I
thought this likely to confuse but if you prefer it I'm happy.
Now done in the attached patch, I also added support for MxNx3 images;
as I suspected these are noticeably slower than MxNx4 images but I
changed my mind and decided it was worth supporting them.
I changed the PIL image output to output from PIL RGBX (alpha is 255) or
RGBA images only when converting to a string and then array. This will
use more memory but, as the conversion to RGBA is done no more than once
and by PIL code (which is probably optimised), should be the fastest
option. I changed the conversion code to always attempt to convert to
RGBA whenever an unsupported format is given and to raise an error on
catching a ValueError (rather than before trying the conversion); this
adds support for several image types. I'd also suggest changing the
error raised here to a ValueError rather than a SystemError (as the
error is due to an unsupported value) but haven't in case others are
catching the SystemError.
Additionally I changed the docstring for the imshow function to state
that floating point MxNx3 and MxNx4 arrays aren't normalised (this was
the original source of my earlier confusion).
Nick
patch (8.03 KB)
···
On Wed, 2005-06-01 at 13:14 +0100, Nicholas Young wrote:
On Wed, 2005-06-01 at 12:39 +0100, Nicholas Young wrote:
> On Tue, 2005-05-31 at 10:28 -0500, John Hunter wrote:
> > I have mixed feelings about making this a separate class / separate
> > function. The current axes imshow / image.AxesImage is already
> > overloaded (handling MxN, MxNx3, MxNx3, _image.Image and PIL images.
> > What is the logic of making a special functions/classes case for MxNx4
> > with directshow. On the one hand, I appreciate the desire to simplify
> > the code by pulling it into separate classes and functions. On the
> > other hand, I wonder if it will confuse users to have one separate
> > function for UInt8 images and another for everything else. Another
> > worry I have about the separate DirectImage class is that it copies
> > much of the image resize functionality from AxesImage, including the
> > broken handling of aspect='preserve'. This too argues for keeping as
> > much functionality in AxesImage as possible, testing for A.typecode()
> > where appropriate. What do you think?
Sorry to reply to my own message but I realised just after sending it
that it didn't make any sense (as MxNx4 images aren't normalised
anyway). I now realise that it makes sense to assume that an MxNx4
UInt8 image will contain pixel data as John suggested and I'll rewrite
my code to implement this.
It also occurred to me that it would make sense to change the PIL code
to avoid the UInt8 -> Float -> UInt transition. I'll try implementing
this after making the alterations John suggested.