scifio / scifio-ome-xml

SCIFIO plug-in providing support for the OME data model, including readers, writers and translators for OME-XML and OME-TIFF formats.
http://scif.io/
BSD 2-Clause "Simplified" License
2 stars 3 forks source link

Crop OME Tiff file loose Plane tags #14

Open hadim opened 10 years ago

hadim commented 10 years ago

Plane tags are not mandatory in OME XML but they can contain very sensitive informations such as time, XYZ position and exposure time.

When an OME Tiff file is cropped, saved and then re open. Plane tags contained in the original file are missing.

Is that a bug or something you don't translate intentionally ?

I report here missing Plane tags because I use them but the same comment could be done for other optionals OME XML metadata.

PS: when I say Save and Open, I only use Legacy mode & Bio-Formats plugin or Modern mode.

hinerm commented 10 years ago

I think this is because the ImgSaver creates a Metadata from the source ImgPlus and uses it to translate - effectively losing any format-specific Metadata (such as OME-XML).

What should happen is that we update ImgSaver to use SCIFIOImgPlus instead, then use translate(Metadata, ImageMetadata) where the Metadata comes from SCIFIOImgPlus.getMetadata() (to include any format-specific metadata) and the ImageMetadata comes from the current SCIFIOImgPlus, as before (to account for updates such as cropping).

hinerm commented 10 years ago

So, I made some changes to SCIFIO that should preserve metadata when writing, but there is a larger problem that I think most operations create new ImgPlus objects, making these scifio-level changes insufficient.

I can't think of a good way to solve this right now, unfortunately. So this ticket may have to remain open for a bit.

hadim commented 10 years ago

Ok I get it Mark. I have a question : why are you using ImgPlus while SCIFIO is becoming standard for I/O in IJ2 ? Can't you use ScifioImgPlus instead ? Maybe you want to separate IJ2 to SCIFIO at some point.

hinerm commented 10 years ago

@hadim I am actually updating SCIFIO code to use SCIFIOImgPlus (for example, the IO class and related ImgOpener/ImgSaver).

However, there are a number of operations that were written before SCIFIOImgPlus existed (especially when you look at IJ1 plugins that are being executed in IJ2) and unfortunately, as far as I can tell, the ImgPlus framework wasn't really designed with subclassing and adding fields in mind.

It definitely has the capacity to work via the copy method, but ultimately that is just a convention you know? There is nothing to force developers to use copy instead of new ImgPlus.

So on our end we could refactor IJ2 commands that use new ImgPlus to use copy when possible, but we would still want to interact positively with older commands or those developed on pure ImgLib2 (and thus without the SCIFIO dependency, thus no SCIFIOImgPlus).

So maybe it was a mistake to just attach Metadata to the SCIFIOImgPlus. Ideally we want a system within SCIFIO itself that will help preserve metadata across all uses of ImgPluses in a backwards-compatible way. So maybe what we want is some sort of Metadata mapping, although that feels over-complicated and still error prone. I will think more on it...

@ctrueden or @dscho do you have any suggestions on how to avoid losing Metadata through operations on ImgPluses without hacking ImgLib2 itself?

hadim commented 10 years ago

Ok I understand the issue here. I let you decide the best way to handle that.

However preserving metadata across image manipulations (even if it cannot be done for every situations I guess) seems to be pretty important to me. I am not talking about supporting metadata propagations on IJ1 plugins. IJ2 plugins support should be sufficient in my opinion.

Last thing you may want to think about : how far do you want to be able to preserve metadata ? First example with Plane tags is pretty easy to handle. But what for example if the user decide to delete on frame over two ? Could you easly handle this case ?

Your choice guys.

Thank you again to take care about all these things and good luck !

ctrueden commented 10 years ago

@hinerm Not sure if this is related, but I notice an oddity with how SCIFIOImgPlus instances were being wrapped; see: https://github.com/scifio/scifio/commit/b21beee1b60d174762497b0dc898fc92f3fa61aa

ctrueden commented 10 years ago

@hinerm wrote:

@ctrueden or @dscho do you have any suggestions on how to avoid losing Metadata through operations on ImgPluses without hacking ImgLib2 itself?

We need to hack ImgLib2 itself. The ImgPlus class needs improvement. Actually, I did add one critical thing just yesterday: https://github.com/imglib/imglib/commit/46feae1f3f253b00ce5f9f9fd2a11e2f42705f98

This addresses a long-standing ticket in the ImageJ Trac, and will likely make it easier for SCIFIO to cache information in an ImgPlus directly.

@hinerm: To be clear: upon reflection, I don't like it that you had to create SCIFIOImgPlus just to add a couple of fields. Better would be to use vanilla ImgPlus objects with information stored in the properties table, now that there is one. We can also make further changes to ImgPlus such as making it into an interface, if that would help you. But as things stand, I fear there is too much wrapping of Img to ImgPlus to SCIFIOImgPlus and things are getting lost in the shuffle.

Not to mention the fact that if the API promises an ImgPlus, but the critical metadata you need is only available in the wrapped SCIFIOImgPlus, which only exists transiently, then there will be no way of accessing said metadata afterward. I ran into this exact issue yesterday because I needed the Metadata created by the SCIFIO Writer and had no way to get it after calling the ImgSaver.