<div dir="ltr">Makes sense. Thanks!</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Mar 11, 2014 at 8:56 AM, Melissa Linkert <span dir="ltr"><<a href="mailto:melissa@glencoesoftware.com" target="_blank">melissa@glencoesoftware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Patrick,<br>
<div class=""><br>
> I noticed that you removed this part of the patch<br>
><br>
> - MetadataTools.populatePixels(store, this);<br>
> + MetadataTools.populatePixels(store, this, true);<br>
><br>
> My read of the code was that was needed to populate the TheC, TheT, TheZ<br>
> values in the Plane objects so that you can actually relate the metadata to<br>
> the correct files. Do I understand this correctly?<br>
<br>
</div>You are correct that adding the 'true' argument is necessary to<br>
correctly populate the Plane objects. That was only removed because it is<br>
introduced in a prior commit in the same open pull request:<br>
<br>
<a href="https://github.com/melissalinkert/bioformats/commit/bd88abca7cbe8dbec99e2ad55c70bf08728dbc62" target="_blank">https://github.com/melissalinkert/bioformats/commit/bd88abca7cbe8dbec99e2ad55c70bf08728dbc62</a><br>
<br>
so including it in both commits would have resulted in a Git merge conflict.<br>
<br>
Regards,<br>
-Melissa<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Mar 10, 2014 at 10:35:56PM -0700, Patrick Riley wrote:<br>
> Thanks Melissa.<br>
><br>
> I noticed that you removed this part of the patch<br>
><br>
> - MetadataTools.populatePixels(store, this);<br>
> + MetadataTools.populatePixels(store, this, true);<br>
><br>
> My read of the code was that was needed to populate the TheC, TheT, TheZ<br>
> values in the Plane objects so that you can actually relate the metadata to<br>
> the correct files. Do I understand this correctly?<br>
><br>
><br>
> On Mon, Mar 10, 2014 at 8:23 PM, Melissa Linkert <<br>
> <a href="mailto:melissa@glencoesoftware.com">melissa@glencoesoftware.com</a>> wrote:<br>
><br>
> > Hi Patrick,<br>
> ><br>
> > > I have a patch attached below that I'd like someone to take a look at.<br>
> > ><br>
> > > In /loci/formats/in/FV1000Reader.java<br>
> > > a bunch of per-plane information is available but not put into the<br>
> > metadata<br>
> > > store (and therefore won't show up in conversions to OME-TIFF for<br>
> > example)<br>
> > ><br>
> > > The attached patch pulls out these fields from the pty files so that you<br>
> > > get Plane fields like this:<br>
> ><br>
> > Thank you very much for the patch. A very slightly modified version of<br>
> > your patch is here:<br>
> ><br>
> ><br>
> > <a href="https://github.com/melissalinkert/bioformats/commit/7528d58f35188147c72ca4bfa2533a6fbb9f05a5" target="_blank">https://github.com/melissalinkert/bioformats/commit/7528d58f35188147c72ca4bfa2533a6fbb9f05a5</a><br>
> ><br>
> > and is included in this open Github pull request for review:<br>
> ><br>
> > <a href="https://github.com/openmicroscopy/bioformats/pull/859" target="_blank">https://github.com/openmicroscopy/bioformats/pull/859</a><br>
> ><br>
> > If anything looks amiss there, please let me know. The only changes<br>
> > from your original patch were to remove the formatting change in<br>
> > FormatTools,<br>
> > fix the units for DeltaT, and add some error handling when parsing the X<br>
> > and<br>
> > Y position values.<br>
> ><br>
> > Once that pull request is marked as being merged, the dev_5_0 branch of<br>
> > Bio-Formats' Git repository will contain this change (and so it will be<br>
> > included in the 5.0.1 release).<br>
> ><br>
> > > A few questions;<br>
> > > * The units for the PositionX, PositionY, PositionZ are not clear to me.<br>
> > > The input format gives it's units, but schema documentation just says<br>
> > > "units are in the microscope reference frame"] and I'm not sure what that<br>
> > > means.<br>
> ><br>
> > All that means is that units are not explicitly defined for those<br>
> > values; they are meant to be stored in OME-XML exactly as they were<br>
> > written in the original file, without any conversion.<br>
> ><br>
> > > * I'm relying on Axis 7 containing the stage information, but I have no<br>
> > > idea if that is always expected. Do you have better documentation?<br>
> ><br>
> > As far as I know, that's correct.<br>
> ><br>
> > > * This works on the files I have, but if there are regression tests for<br>
> > > this format somewhere that need to be updated, I haven't found them (and<br>
> > > it's not clear to me how to run this, so pointers appreciated).<br>
> ><br>
> > Data regression tests are run automatically against all commits included<br>
> > in open Github PRs, the results of which are displayed here:<br>
> ><br>
> > <a href="http://ci.openmicroscopy.org/view/Bio-Formats" target="_blank">http://ci.openmicroscopy.org/view/Bio-Formats</a><br>
> ><br>
> > As a patch submitter, though, that's typically not something you need to<br>
> > worry too much about - anything that goes wrong there will be caught by<br>
> > the code reviewer(s).<br>
> ><br>
> > Regards,<br>
> > -Melissa<br>
> ><br>
> > On Fri, Mar 07, 2014 at 08:16:55AM -0800, Patrick Riley wrote:<br>
> > > I have a patch attached below that I'd like someone to take a look at.<br>
> > ><br>
> > > In /loci/formats/in/FV1000Reader.java<br>
> > > a bunch of per-plane information is available but not put into the<br>
> > metadata<br>
> > > store (and therefore won't show up in conversions to OME-TIFF for<br>
> > example)<br>
> > ><br>
> > > The attached patch pulls out these fields from the pty files so that you<br>
> > > get Plane fields like this:<br>
> > ><br>
> > > <Plane DeltaT="0.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4184E7" TheC="0" TheT="0" TheZ="0"/><br>
> > > <Plane DeltaT="0.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4184E7" TheC="1" TheT="0" TheZ="0"/><br>
> > > <Plane DeltaT="0.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4184E7" TheC="2" TheT="0" TheZ="0"/><br>
> > > <Plane DeltaT="1512.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4187E7" TheC="0" TheT="0" TheZ="1"/><br>
> > > <Plane DeltaT="1512.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4187E7" TheC="1" TheT="0" TheZ="1"/><br>
> > > <Plane DeltaT="1512.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4187E7" TheC="2" TheT="0" TheZ="1"/><br>
> > > <Plane DeltaT="3025.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.419E7" TheC="0" TheT="0" TheZ="2"/><br>
> > > <Plane DeltaT="3025.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.419E7" TheC="1" TheT="0" TheZ="2"/><br>
> > > <Plane DeltaT="3025.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.419E7" TheC="2" TheT="0" TheZ="2"/><br>
> > > <Plane DeltaT="4537.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4193E7" TheC="0" TheT="0" TheZ="3"/><br>
> > > <Plane DeltaT="4537.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4193E7" TheC="1" TheT="0" TheZ="3"/><br>
> > > <Plane DeltaT="4537.0" PositionX="3.1997411E7"<br>
> > > PositionY="4.0177623E7" PositionZ="2.4193E7" TheC="2" TheT="0" TheZ="3"/><br>
> > ><br>
> > > A few questions;<br>
> > > * The units for the PositionX, PositionY, PositionZ are not clear to me.<br>
> > > The input format gives it's units, but schema documentation just says<br>
> > > "units are in the microscope reference frame"] and I'm not sure what that<br>
> > > means.<br>
> > > * I'm relying on Axis 7 containing the stage information, but I have no<br>
> > > idea if that is always expected. Do you have better documentation?<br>
> > > * This works on the files I have, but if there are regression tests for<br>
> > > this format somewhere that need to be updated, I haven't found them (and<br>
> > > it's not clear to me how to run this, so pointers appreciated).<br>
> > > * I also haven't programmed in Java for a while, so if I've done random<br>
> > > stuff wrong, please let me know!<br>
> > ><br>
> > > Thanks.<br>
> > > --<br>
> > > Patrick Riley<br>
> > > <a href="mailto:pfr@google.com">pfr@google.com</a><br>
> ><br>
> ><br>
> > > _______________________________________________<br>
> > > ome-devel mailing list<br>
> > > <a href="mailto:ome-devel@lists.openmicroscopy.org.uk">ome-devel@lists.openmicroscopy.org.uk</a><br>
> > > <a href="http://lists.openmicroscopy.org.uk/mailman/listinfo/ome-devel" target="_blank">http://lists.openmicroscopy.org.uk/mailman/listinfo/ome-devel</a><br>
> ><br>
> ><br>
><br>
><br>
> --<br>
> Patrick Riley<br>
> <a href="mailto:pfr@google.com">pfr@google.com</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>Patrick Riley<br><a href="mailto:pfr@google.com" target="_blank">pfr@google.com</a><br>
</div>