Tobias Wood wrote: > Michael, > Thanks for the comments, much appreciated. I've attached an updated > patch including your suggestions and some more whitespace for > readability. There was no reason other than simplicity for not > returning a 8-bit numpy array. I actually meant to ask about the > ternary blocks - I think I picked up the style from the original code > and had continued in the same vein for compactness. Thanks! > > While I was testing this I came across another issue - which variety > of FLOAT should the code return? My understanding was that Python > floats are C doubles. However the code was previously returning a > PyArray_FLOAT, which seems to be a FLOAT32 rather than a FLOAT64. > Hence I removed any trace of doubles from my code and have left it all > at float precision. Yes, that's all correct. I suspect read_png creates arrays of floats rather than doubles just for the sake of memory savings -- and the fact that doubles would be overkill for even 16-bit integral data.
Thanks for the new patch. I'll wait a bit to see if there's any comments on the functionality itself or the API before committing it. Cheers, Mike > > Thanks again, > Toby > > On Mon, 22 Jun 2009 15:58:58 +0100, Michael Droettboom > <md...@stsci.edu> wrote: > >> I don't ever work with data-in-PNGs, so I won't comment on the use >> cases or API here -- I'll leave that to others. >> >> However, for the patch, I think the reinterpret_cast<unsigned short >> *> would be safer as reinterpret_cast<png_uint_16>, since unsigned >> ints are not guaranteed to be 16-bits, and png.h provides a nice >> convenient typedef for us. Also, why does the code not create an >> 8-bit numpy array for "raw" images that are only 8-bits? >> >> Also a style note: I find assignments inside of ternary operators >> (... ? ... : ...) confusing. I'd rather see that as a proper "if" >> block. >> >> Cheers, >> Mike >> >> Tobias Wood wrote: >>> Dear list, >>> Back in April I submitted a patch that allowed imread() to correctly >>> read PNGs that have odd bit-depths, ie not 8 or 16 (I actually >>> submitted that to the Users list as I was unsure of protocol). There >>> were a couple of things I left unfinished that I've finally got >>> round to looking at again. >>> >>> The main remaining issue for me is that PNG specifies that all bit >>> depths should be scaled to have the same maximum brightness, so that >>> a value of 8191 in an 13-bit image is displayed the same as 65535 in >>> a 16-bit image. Unfortunately, the LabView drivers for the 12-bit >>> CCD in our lab do not follow this convention. A higher bit-depth >>> from this setup means the image was brighter in an absolute sense >>> and no scaling takes place. So this is not an error with Matplotlib >>> as such, but more about having a decent way to handle iffy PNGs. It >>> is worth noting that Matlab does not handle these PNGs well either >>> (We have to query the image file using iminfo and then correct it) >>> and PIL ignores anything above 8-bits as far as I can tell. >>> >>> A simple method, in my mind, and originally suggested by Andrew >>> Straw is to add a keyword argument to imread() that indicates >>> whether a user wants floats scaled between 0 and 1, or the raw byte >>> values which they can then scale as required. This then gets passed >>> to read_png(), which does the scaling if necessary and if not >>> returns an array of UINT16s. I wrote a patch that does this, >>> changing both image.py and _png.cpp. I'm very much open to other >>> suggestions, as I didn't particularly want to fiddle with a core >>> function like imread() and I'm fairly new to Python. In particular I >>> have not changed anything to do with PIL - although it would not be >>> much work to update pil_to_array() to follow the same behaviour as >>> read_png(). I have tested this with the pngsuite.py*, and if desired >>> I can submit an extended version of this that tests the extended >>> bit-depth images from the PNG suite. >>> >>> Thanks in advance, >>> Toby Wood >>> >>> * My patch also includes a minor change to pngsuite.py which was >>> throwing a deprecation warning about using get_frame() istead of patch >>> ------------------------------------------------------------------------ >>> >>> >>> ------------------------------------------------------------------------------ >>> >>> >>> Are you an open source citizen? Join us for the Open Source Bridge >>> conference! >>> Portland, OR, June 17-19. Two days of sessions, one day of >>> unconference: $250. >>> Need another reason to go? 24-hour hacker lounge. Register today! >>> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org >>> >>> >>> ------------------------------------------------------------------------ >>> >>> >>> _______________________________________________ >>> Matplotlib-devel mailing list >>> Matplotlib-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel >> > -- Michael Droettboom Science Software Branch Operations and Engineering Division Space Telescope Science Institute Operated by AURA for NASA ------------------------------------------------------------------------------ Are you an open source citizen? Join us for the Open Source Bridge conference! Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250. Need another reason to go? 24-hour hacker lounge. Register today! http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org _______________________________________________ Matplotlib-devel mailing list Matplotlib-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
