Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug-fix: wrong metadata used for scale in ZEISS tifs #2056

Closed
wants to merge 1 commit into from
Closed

bug-fix: wrong metadata used for scale in ZEISS tifs #2056

wants to merge 1 commit into from

Conversation

sem-geologist
Copy link
Contributor

@sem-geologist sem-geologist commented Sep 14, 2018

Current problem

Currently loading 0.5k, 2k and 3k resolution zeiss tif files gets wrong scale.

Detailed description

Zeiss SEM can produce different resolution images. There is one unfortunate detail, tag contains two parameters for resolution: AP_pixel_size and AP_image_pixel_size, where currently first one is used in hyperspy. If image is taken at 1k resolution, both parameters are the same, however if 0.5k, 2k or 3k, those attributes differ (2x, 0.5x, 0.(3)x). AP_pixel_size does not represent the real pixel size of the image, but only physical pixel size on physical monitor. AP_image_pixel_size however presents the real pixel size.

Description of the change

Change the attribute for scale from AP_pixel_size to AP_image_pixel_size so that 2k and 3k images could be load with the right scale.

Progress of the PR

  • Change implemented,
  • ready for review.

I am not sure it would be good to upload 2k zeiss image (additional 3MB) to the test folder, current test should be enough.

for zeiss tiff reader
@sem-geologist
Copy link
Contributor Author

Additional problems found (for sure not for this PR)

I have found also that hyperspy has the problem with getting time and date. I tracked the problem and it is originating from Tifffile library... well, not exactly, the Time and Date gets not parsed by Gohlke's Tifffile library as it is for some unknown reasons saved inconsistently inside Zeiss tag. Most of tags have key and argument separated by "=", but date and time with ":".
I tried to put make fix locally on my computer (on Tifffile lib), and this needs only few line of code.
this:

try:
    name, value = line.split('=')
except ValueError:
    continue

when is changed with this:

try:
     name, value = line.split('=')
except ValueError:
      try:
           name, value = line.split(':', 1)
      except :
            continue

I am wondering if that is optimal way...
I will try to send the patch to Gohlke, and then I need to convince skimage to update the library. Only then it makes sense to add/uncomment relevant part in hyperspy's tiff.py.

@ericpre
Copy link
Member

ericpre commented Sep 17, 2018

I remember to have been annoyed with this and I could not sorted out because I have been provided with only one file!
If this is not possible to make a very smaller image on a Zeiss SEM, I would agree that it is not worth adding an image to test only this.

Could you please open an separate issue on the date/time bug, since this is going to take a bit of time to get sorted out?

@sem-geologist
Copy link
Contributor Author

Ok, this than can be reviewed.

@ericpre
Copy link
Member

ericpre commented Sep 25, 2018

Some Zeiss tiff file doesn't have the AP_image_pixel_size metadata key... see file:
https://www.dropbox.com/s/i0prg10yfktnlu2/A600_04.tif?dl=0

A 512 pixel image is ~200kB, so it should be fine to add it to the test suite?

@sem-geologist
Copy link
Contributor Author

interesting... I need to meditate this :)
...
@ericpre , thanks for opening some can of worms :P.

Well this is not worse case actually, the even older versions had no human readable key-value tags, but list of tuples with some essential and not so essential metadata, which is still prepending the readable part even in the latest format.
So I guess You force me to put some if/else loop, for fall back methods in getting the right scale even for older versions of zeiss-gemini tiffs. I only had heard about some older formats, but now you had put one example in front of me – I have no choice but to expand this PR. Can this shared file be used/placed as one more test file?

@sem-geologist
Copy link
Contributor Author

One can get to lean for the the logic like this :

  1. check if ap_image_pixel_size is in tags: direct scale value
  2. if not, check ap_pixel_size: not so direct_scale_value as it needs knowledge about the dpi of monitor
    alternative way is to search for ap_width and ap_height and divide those by image pixel dimensions that gives much more reliable value.
  3. if no readable tags available (no ap_image_pixel_size,ap_pixel_size, ap_width) use pixel_size from poorly document list of tuples/lists which always is coming before the human readable key-value-units part.
    (this kind of logic is used i.e. by Gwyddion, which was set so BTW after I had reported that pixel_size is not always the right value)

But, considering the above shared image sample...
Personally I would just ignore pixel_image_size and pixel_size or hacky way of ap_width/pixels_image_width. Personally I am convinced that the most precise way is to use the value from undocumented parameter tuple (which have much better precision than human readable forms). Thanks for this fine gemini-zeiss tiff sample from 2010 you had shared I can finally put aside all my doubts and clearly conclude that to get the real universal real pixel_size we need to extract the pixel_size from poorly_documented tuple which needs to be multiplied by 1k resolution normalized with the real image resolution (that is factor as i.e. 1024/2048= 0.5; 1024/512 = 2...). This I think is the most elegant way without unnecessary if/else conditions.

@sem-geologist
Copy link
Contributor Author

sem-geologist commented Sep 25, 2018

I am thinking if not to close this PR, as I am considering to push PR for #2057 with complete update of Tifffiles, where lots of tiff.py in io_plugins changes, as newest tifffiles simplifies lots of things a lot. So I could shoot the different problems with Zeiss tif reading in single PR. Unless that PR will be fiercely rejected, then I should implement it with this PR. I am nearly done with test adoption for most recent tifffile. I fear simply that merging next_patch and next_minor could get quite painful considering zeiss tifs.

@ericpre
Copy link
Member

ericpre commented Sep 26, 2018

Personally I would just ignore pixel_image_size and pixel_size or hacky way of ap_width/pixels_image_width. Personally I am convinced that the most precise way is to use the value from undocumented parameter tuple (which have much better precision than human readable forms). Thanks for this fine gemini-zeiss tiff sample from 2010 you had shared I can finally put aside all my doubts and clearly conclude that to get the real universal real pixel_size we need to extract the pixel_size from poorly_documented tuple which needs to be multiplied by 1k resolution normalized with the real image resolution (that is factor as i.e. 1024/2048= 0.5; 1024/512 = 2...). This I think is the most elegant way without unnecessary if/else conditions.

This is exactly what I was hoping for: find a more universal approach! And if you work something out then, that's great! :)

Can this shared file be used/placed as one more test file?

Yes, you can use this file, it is not mine but I have asked for permission.

I am thinking if not to close this PR, as I am considering to push PR for #2057 with complete update of Tifffiles, where lots of tiff.py in io_plugins changes, as newest tifffiles simplifies lots of things a lot.

Up to you, what you think it is more sensible/convenient.

@sem-geologist
Copy link
Contributor Author

I am closing this as PR #2064 addresses this issue better.

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

Successfully merging this pull request may close these issues.

None yet

3 participants