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

tifffile plugin: Various fixes #567

Merged
merged 5 commits into from
Nov 20, 2020
Merged

Conversation

lschr
Copy link
Contributor

@lschr lschr commented Nov 17, 2020

Reader.get_meta_data(i) already worked correctly, but Reader.get_data(i).meta contained the result of the last call to Reader.get_meta_data regardless of i.

Writer.append_data(img, meta) set the global metadata to meta, which was also not what I would expect.

Here is a possible fix which hopefully does not break anything. With this, any frame written to a file using append_data(…, meta=None) will have the global metadata associated with it. This is debatable but resembles the old behavior. The alternative would be to write the global metadata only for frame 0 and no metadata for other frames unless explicitly given—which I think would actually make more sense.

@coveralls
Copy link

coveralls commented Nov 17, 2020

Coverage Status

Coverage increased (+0.006%) to 88.99% when pulling 23ebabb on lschr:tiffstack_metadata into 182129b on imageio:master.

@lschr
Copy link
Contributor Author

lschr commented Nov 17, 2020

Thinking of it, I see two options for reading metadata:

  1. Use only the frame's metadata for each frame (currently implemented).
  2. Take the global metadata dict and update that with frame's metadata for each frame (not implemented).

@almarklein
Copy link
Member

With this, any frame written to a file using append_data(…, meta=None) will have the global metadata associated with it. This is debatable but resembles the old behavior. The alternative would be to write the global metadata only for frame 0 and no metadata for other frames unless explicitly given—which I think would actually make more sense.

I think you're right here.

Does Tiff have a notion of global meta data, or would this come down to the metadata of the first image.

In get_meta_data(), when index is None it should return the global metadata. If that does not exist, the data at 0 makes the most sense.

Thinking of it, I see two options for reading metadata:

I think option 1 is best, since there is a way to read the global meta data. (Albeit this might perhaps be documented better.)

Lukas Schrangl added 3 commits November 20, 2020 10:02
and only if no metadata was given for first frame.
This has been tifffile's default since version 2020.9.3 and allows for
e.g. per-frame metadata.
if available. `save` was deprecated in tifffile version 2020.9.30.
@lschr lschr changed the title tiffile plugin: Don't assume metadata is the same for all frames tiffile plugin: Various fixes Nov 20, 2020
@lschr
Copy link
Contributor Author

lschr commented Nov 20, 2020

I don't know much about TIFF, but as far as I gather from tifffile, there is no real global metadata, but the first frame is used. As of now, global metadata will be written with the first frame, but only if no metadata was passed to append_data for the first frame. I clarified that in the documentation.

@lschr
Copy link
Contributor Author

lschr commented Nov 20, 2020

I fixed two more things:

  • As TiffWriter.save has been deprecated, use the new TiffWriter.write if available
  • Explicitly set contiguous=False when saving. This is the default in tifffile nowadays and (AFAICS) treats the saved images more independently, e.g., allows for per-frame metadata, compression, and so on. Changed the write-read test to make sure that per-frame metadata works.

@lschr lschr changed the title tiffile plugin: Various fixes tifffile plugin: Various fixes Nov 20, 2020
@almarklein
Copy link
Member

Looking good. Thanks @lschr !

@almarklein almarklein merged commit 6d8a0b7 into imageio:master Nov 20, 2020
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