7856 flim ann #820

joshmoore · 2013-12-05T16:25:52Z

This branch propagates the XMLAnnotation created by Bio-Formats on reading the FLIM settings of .sdt file. The block should be equivalent to the xml found by running:

bfconvert foo.sdt foo.ome.tiff
tiffcomment foo.ome.tiff > foo-raw.ome.xml
xmllint --format foo-raw.ome.xml > foo.ome.xml

The OMERO annotation can typically be found with:

omero import foo.sdt
psql omero -c 'select textvalue from annotation order by id desc limit 1'

Current issues with this PR:

It seems to have broken the bfconvert usage which quite critical ("Caused by: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 13; The processing instruction target matching "[xX][mM][lL]" is not allowed.")
~~Namespace handling in the Annotation blocks is not correct. ("warning : xmlns: URI openmicroscopy.org/.../modulo is not absolute" is printed by xmllint).~~ The value generated should match https://www.openmicroscopy.org/site/support/ome-model/developers/6d-7d-and-8d-storage.html
~~The API changes to OMEXMLMetadata are likely not appropriate.~~ (rolled back)

/cc @melissalinkert @jburel

This is largely a cleanup of the three copies of modulo creation for Z, T, and C. Notable is the call to Image.linkAnnotation which was not previously being called. Without this, OMERO is not aware of the annotation.

This is a suboptimal solution since it will require other interface implementors to also implement the new methods, but this was the simplest way to provide the necessary XML parsing for individual elements.

setValue() on the Modulo object was causing bfconvert to fail due to XML parsing. This has now been disabled. At the same time the NS elements from asXMLElement() have been removed creating the desired block of XML: ``` <XMLAnnotation ID="Annotation:90" Namespace="openmicroscopy.org/omero/dimension/modulo"> <Value> <ModuloAlongT End="12304.687643793777" Start="0.0" Step="195.3125022824409" Type="lifetime" TypeDescription="TCSPC" Unit="ps"/> </Value> </XMLAnnotation> ```

XMLTools now permits disabling the `<? xml` block on writeXML and dumpXML which allows us to move a step further, though bfconvert is still failing with: ``` Exception in thread "main" java.lang.RuntimeException: loci.common.services.ServiceException: java.lang.RuntimeException: Value node list size 2 != 1 ```

Note: the private 'value' field hides the proper xmlannotation 'value' field which is a bit confusing.

joshmoore · 2013-12-06T14:27:56Z

With the committed changes, the bottom of the tiffcomment (xmllinted) looks like this:

    <XMLAnnotation ID="Annotation:89" Namespace="openmicroscopy.org/OriginalMetadata">
      <Value>
        <OriginalMetadata>
          <Key>MeasureInfo.cfdZC</Key>
          <Value>-8.3149605</Value>
        </OriginalMetadata>
      </Value>
    </XMLAnnotation>
    <XMLAnnotation ID="Annotation:90" Namespace="openmicroscopy.org/omero/dimension/modulo">
      <Value>
        <Modulo namespace="http://www.openmicroscopy.org/Schemas/Additions/2011-09">
          <ModuloAlongT End="12304.687643793777" Start="0.0" Step="195.3125022824409" Type="lifetime" TypeDescription="TCSPC" Unit="ps"/>
        </Modulo>
      </Value>
    </XMLAnnotation>
  </StructuredAnnotations>
</OME>

melissalinkert · 2013-12-06T16:08:35Z

components/scifio/src/loci/formats/ome/OMEXMLMetadata.java

@@ -36,6 +36,9 @@

 package loci.formats.ome;

+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+


Unnecessary imports?

fix pushed.

melissalinkert · 2013-12-06T16:56:34Z

+1 otherwise. Importing an .sdt file into OMERO does show that the modulo annotation is being stored/retrieved correctly, as the slider shows "t" and the corresponding value in ps. I will need to fix some parsing problems in OMEXMLMetadataImpl.getModuloAlong(...) noticed while trying to read a converted OME-TIFF, but that is not this PR's fault so can happen after this is merged.

melissalinkert · 2013-12-07T00:17:19Z

This does cause some jobs to fail, e.g. http://hudson.openmicroscopy.org.uk/view/Bio-Formats/job/BIOFORMATS-test_images_good/659 with errors similar to:

INFO [pool-1-thread-5] [06-12-2013 22:12:31.108] TiffDelegateReader initializing /ome/data_repo/test_images_good/metamorph/jae/RGB_s34.tif
ERROR [pool-1-thread-5] [06-12-2013 22:12:31.108] 
java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
    at java.util.ArrayList.rangeCheck(ArrayList.java:604)
    at java.util.ArrayList.get(ArrayList.java:382)
    at ome.xml.model.OME.getImage(OME.java:647)
    at loci.formats.services.OMEXMLServiceImpl.addModuloAlong(OMEXMLServiceImpl.java:866)
    at loci.formats.FormatReader.setId(FormatReader.java:1397)
    at loci.formats.DelegateReader.setId(DelegateReader.java:252)
    at loci.formats.ImageReader.setId(ImageReader.java:781)
    at loci.formats.ReaderWrapper.setId(ReaderWrapper.java:576)
    at loci.formats.Memoizer.setId(Memoizer.java:471)
    at loci.formats.ReaderWrapper.setId(ReaderWrapper.java:576)
    at loci.formats.DimensionSwapper.setId(DimensionSwapper.java:301)
    at loci.formats.FileStitcher.initFile(FileStitcher.java:878)
    at loci.formats.FileStitcher.setId(FileStitcher.java:819)
    at loci.formats.ReaderWrapper.setId(ReaderWrapper.java:576)
    at loci.tests.testng.FormatReaderTest.initFile(FormatReaderTest.java:2189)
    at loci.tests.testng.FormatReaderTest.initFile(FormatReaderTest.java:2140)
    at loci.tests.testng.FormatReaderTest.testBufferedImageDimensions(FormatReaderTest.java:167)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:601)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:80)
    at org.testng.internal.Invoker.invokeMethod(Invoker.java:714)
    at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:901)
    at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1231)
    at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:127)
    at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:111)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
    at java.lang.Thread.run(Thread.java:722)

For /ome/data_repo/test_images_good/metamorph/jae/RGB_s34.tif `getImage(imageIdx)` throws an IndexOutOfBounds exception since there are no images detected. In this case, modulo handling is exiting prematurely since there's no image to link to.

joshmoore · 2013-12-09T08:59:39Z

Removing exclude so this will be in the next set of BF tests.

In order to be properly tested, these classes need to be made public. This is also the beginning of the definition of their public API which will also be further defined via ticket 4536

jburel · 2013-12-10T21:45:31Z

problem reported by @imunro.
I tried a couple. One of Andrew's examples & one I created by exporting one of our images from our 4.4.9 server.
Both look, in insight, as if they have 2 identical xmlAnnotations.
See Gretzky user-1 => xmlAnnotationTest->misc-> *.ome.tif for examples.

The image has only one XML annotation

One is generated and one is read from the file itself. 2 annotations with different ids end up being linked to the image.

imunro · 2013-12-10T21:56:02Z

jburel's comment above refers to importing one-tiffs via insight.

jburel · 2013-12-10T21:58:54Z

The plan is to replace angle by rotation in the model since it is the value currently used in czi
(!type.equals("angle") && !type.equals("phase") && !type.equals("tile") && !type.equals("lifetime") && !type.equals("lambda")))
Do we want to support that already here? i.e. if (type.equals("rotation") type = "angle"
so it does not end up as "other"

joshmoore · 2013-12-11T08:36:07Z

"rotation" was also requested in @rleigh-dundee's http://trac.openmicroscopy.org.uk/ome/ticket/11720

See also http://trac.openmicroscopy.org.uk/ome/ticket/11721

We'll need to discuss specific next steps today.

jburel · 2013-12-11T08:44:40Z

@joshmoore: The ticket is for the same reason, I did not have access to trac to find the number

Following exception was thrown for start=0.0 when converting an ome.tiff with modulo annotation back to ome.tiff: ``` Populating metadata Exception in thread "main" java.lang.NumberFormatException: For input string: "0.0" at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65) at java.lang.Integer.parseInt(Integer.java:492) at java.lang.Integer.parseInt(Integer.java:527) at loci.formats.services.OMEXMLServiceImpl.getModuloAlong(OMEXMLServiceImpl.java:564) at loci.formats.services.OMEXMLServiceImpl.getModuloAlongT(OMEXMLServiceImpl.java:528) at loci.formats.in.OMETiffReader.initFile(OMETiffReader.java:834) at loci.formats.FormatReader.setId(FormatReader.java:1360) at loci.formats.ImageReader.setId(ImageReader.java:781) at loci.formats.tools.ImageConverter.testConvert(ImageConverter.java:337) at loci.formats.tools.ImageConverter.main(ImageConverter.java:672) ```

Previously, when trying: ``` ./bfconvert -no-upgrade -overwrite FLIM-modulo-sample.ome.tiff /tmp/flim.ome.tiff ``` Two modulo annotation blocks appear in the output TIFF comment. With this commit, later calls to addModulo() are silently ignored.

joshmoore · 2013-12-11T11:19:49Z

Tested with the CZI from QA 7555:

    <XMLAnnotation ID="Annotation:198" Namespace="openmicroscopy.org/omero/dimension/modulo">
      <Value>
        <Modulo namespace="http://www.openmicroscopy.org/Schemas/Additions/2011-09">
          <ModuloAlongZ Type="angle" TypeDescription="">
            <Label>7.211999999999988200000000000000</Label>
            <Label>46.783000000000015000000000000000</Label>
            <Label>82.651000000000010000000000000000</Label>
            <Label>118.692000000000010000000000000000</Label>
            <Label>151.246000000000010000000000000000</Label>
          </ModuloAlongZ>
        </Modulo>
      </Value>
    </XMLAnnotation>
    <XMLAnnotation ID="Annotation:199" Namespace="openmicroscopy.org/omero/dimension/modulo">
      <Value>
        <Modulo namespace="http://www.openmicroscopy.org/Schemas/Additions/2011-09">
          <ModuloAlongT Type="phase" TypeDescription="">
            <Label>0.000000000000000000000000000000</Label>
            <Label>72.000000000000000000000000000000</Label>
            <Label>144.000000000000000000000000000000</Label>
            <Label>216.000000000000000000000000000000</Label>
            <Label>288.000000000000000000000000000000</Label>
          </ModuloAlongT>
        </Modulo>
      </Value>
    </XMLAnnotation>

melissalinkert · 2013-12-12T15:22:13Z

This all seems fine - happy to merge. @jburel/@imunro, any objections?

imunro · 2013-12-12T15:27:23Z

No objections from me.

jburel · 2013-12-12T18:16:04Z

good to go

7856 flim ann

joshmoore · 2013-12-16T07:55:34Z

--no-rebase

joshmoore added 8 commits December 5, 2013 17:17

Refactor and link modulo annotation creation

91b2194

This is largely a cleanup of the three copies of modulo creation for Z, T, and C. Notable is the call to Image.linkAnnotation which was not previously being called. Without this, OMERO is not aware of the annotation.

Fill OME model objects

a8f6462

This is a suboptimal solution since it will require other interface implementors to also implement the new methods, but this was the simplest way to provide the necessary XML parsing for individual elements.

Refactor methods into XMLTools and remove API changes

fce4807

Remove asXMLElement to prevent dupe Values

92c8e38

Wrap ModuloAlong element with Modulo

9161817

Also fix OriginalMetadata annotations in the same way

b8c6415

Note: the private 'value' field hides the proper xmlannotation 'value' field which is a bit confusing.

Fix unit test by clarifying getValue v. getValueForKey

5826d45

melissalinkert reviewed Dec 6, 2013
View reviewed changes

Fix unused imports

41f2fa7

Handle missing images

55af2fe

For /ome/data_repo/test_images_good/metamorph/jae/RGB_s34.tif `getImage(imageIdx)` throws an IndexOutOfBounds exception since there are no images detected. In this case, modulo handling is exiting prematurely since there's no image to link to.

Make OM and M Annotation classes public

864efeb

In order to be properly tested, these classes need to be made public. This is also the beginning of the definition of their public API which will also be further defined via ticket 4536

joshmoore added 3 commits December 11, 2013 10:20

Temporary fix for rotation (See #11720)

6a3704c

melissalinkert added a commit that referenced this pull request Dec 12, 2013

Merge pull request #820 from joshmoore/7856-flim-ann

2caab02

7856 flim ann

melissalinkert merged commit 2caab02 into ome:develop Dec 12, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7856 flim ann #820

7856 flim ann #820

joshmoore commented Dec 5, 2013

joshmoore commented Dec 6, 2013

melissalinkert Dec 6, 2013

joshmoore Dec 6, 2013

melissalinkert commented Dec 6, 2013

melissalinkert commented Dec 7, 2013

joshmoore commented Dec 9, 2013

jburel commented Dec 10, 2013

imunro commented Dec 10, 2013

jburel commented Dec 10, 2013

joshmoore commented Dec 11, 2013

jburel commented Dec 11, 2013

joshmoore commented Dec 11, 2013

melissalinkert commented Dec 12, 2013

imunro commented Dec 12, 2013

jburel commented Dec 12, 2013

joshmoore commented Dec 16, 2013

7856 flim ann #820

7856 flim ann #820

Conversation

joshmoore commented Dec 5, 2013

joshmoore commented Dec 6, 2013

melissalinkert Dec 6, 2013

Choose a reason for hiding this comment

joshmoore Dec 6, 2013

Choose a reason for hiding this comment

melissalinkert commented Dec 6, 2013

melissalinkert commented Dec 7, 2013

joshmoore commented Dec 9, 2013

jburel commented Dec 10, 2013

imunro commented Dec 10, 2013

jburel commented Dec 10, 2013

joshmoore commented Dec 11, 2013

jburel commented Dec 11, 2013

joshmoore commented Dec 11, 2013

melissalinkert commented Dec 12, 2013

imunro commented Dec 12, 2013

jburel commented Dec 12, 2013

joshmoore commented Dec 16, 2013