Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Nov 14, 2012

Depends on: PR152

Add copy constructors and clone() methods to replace the existing
copy method. This permits core metadata of unknown derived type
to be safely cloned. Copying is performed directly using copy
constructors. This also permits the removal of the generic
copyCoreMetadata method in ReaderWrapper. getCoreMetadata in
ReaderWrapper may now simply clone() a copy of the core metadata,
and setId in DimensionSwapper can use the DimensionSwapper copy
constructor to do a copy to derive a copy of the core metadata.

Roger Leigh and others added 18 commits September 20, 2012 14:32
setSeries and getSeries work by using coreIndex to compute the correct
value (replaces series), while getCoreIndex and setCoreIndex use the
coreIndex directly.  Readers using series directly have been fixed to
use getSeries.

toSeries and toCoreIndex methods convert between series and core index
numbers, and handle resolution flattening.  All functions dealing with
series numbering use these to handle conversion.
Allow use of both the
  http://www.leica-microsystems.com/scn/2010/03/10 and
  http://www.leica-microsystems.com/scn/2010/10/01
schemas.

- In order to handle both schemas identically, the pixels element
  sizeR/sizeZ/sizeC/firstIFD attributes are no longer parsed; instead
  the sizes for R, Z and C are computed from the dimensions elements.
  We do require that the dimensions are a non-sparse array.
- Fix data order as XYCZT (this matches the existing test cases).
- Thumbnails:
  + Correct IFD mapping for thumbnails
  + Get thumbnail size from correct image
  + Use flattened resolutions when getting thumbnails
Warn if the schema is unsupported, but otherwise try to read it.
This required adding toSeries and toCoreIndex to IFormatReader
(and hence ImageReader).
Subresolutions are not supported.  This removes the need for
series ↔ coreIndex conversion, since they are always the same.
These override the ReaderWrapper implementation, and return their
argument (since subresolutions are not supported).
This enables getSeries to be O(1) in addition to coreIndex.
seriesToCoreIndex and coreIndexToSeries are also O(1) when
the current series/coreIndex matches their arguments.

FileStitcher also has a series member, but this is currently
unused; it was added purely to match the FormatReader behaviour.
The existing constructor using a reader and series number has
two problems:
- it's unsuitable when using non-flattened resolutions since
  one can't directly specify the subresolution
- getSeriesCount will fail if the core[] array is not yet
  fully initialised, making this unusable when using during
  initialisation and strictly validating core[]

Here, a new (simple) copy contructor that does a simple copy
of the CoreMetadata is provided, and this is used directly
in all readers.  This is safe since they are copying their
own metadata, so avoiding the reader interface is not a problem
here.

We also update the existing constructor to use a core index
instead of series, so that it will work for all readers,
whether flat or nonflat, and internally we use setCoreIndex
and getCoreIndex.
Add copy constructors and clone() methods to replace the
existing copy method.  This permits core metadata of
unknown derived type to be safely cloned.  Copying is
performed directly using copy constructors.  This also
permits the removal of the generic copyCoreMetadata
method in ReaderWrapper.  getCoreMetadata in ReaderWrapper
may now simply clone() a copy of the core metadata, and
setId in DimensionSwapper can use the DimensionSwapper
copy constructor to do a copy to derive a copy of the
core metadata.
@ghost
Copy link
Author

ghost commented Nov 14, 2012

Fails tests for jpeg2000 and some related formats, so will need some further work.

@ghost
Copy link
Author

ghost commented Nov 14, 2012

All tests passing now; other than a single pyramid failure picked up from the leicascn-newschema code. The earlier failure was a missbuild.

@ghost
Copy link
Author

ghost commented Nov 14, 2012

Note that the failure here is the rounding issue we discussed last week.

@joshmoore
Copy link
Member

OMERO-merge-develop job 45 failed with:

``
----------=========== scifio ===========----------

copy-source:
Copying 10 files to /home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/src
Copying 208 files to /home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/src
Copying 12 files to /home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/classes
Copied 14 empty directories to 10 empty directories under /home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/classes

compile:
Compiling 198 source files to /home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/classes
/home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/src/loci/formats/CoreMetadata.java:217: CoreMetadata(loci.formats.CoreMetadata) is already defined in loci.formats.CoreMetadata
public CoreMetadata(CoreMetadata template) {
^
/home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/src/loci/formats/CoreMetadata.java:230: cannot find symbol
symbol : method copyOf(int[],int)
location: class java.util.Arrays
Arrays.copyOf(template.cLengths, template.cLengths.length);
^
/home/omero/slave/workspace/OMERO-merge-develop/src/components/bioformats/components/scifio/build/src/loci/formats/CoreMetadata.java:232: cannot find symbol
symbol : method copyOf(java.lang.String[],int)
location: class java.util.Arrays
Arrays.copyOf(template.cTypes, template.cTypes.length);
``

Marking broken.

@joshmoore joshmoore mentioned this pull request Nov 15, 2012
@melissalinkert
Copy link
Member

ant test generates one failure; see http://hudson.openmicroscopy.org.uk/job/BIOFORMATS-per-commit-test/ws/test-output/SCIFIO%20Unit%20Tests/index.html

Removing from builds until that is resolved.

@ghost
Copy link
Author

ghost commented Nov 19, 2012

I don't think the "ant clean" test failure is due to this PR. It's also failing with current develop.

I've merged this into current develop for testing, and the only failing test is for the OME-XML thumbnails (which is expected).

@joshmoore
Copy link
Member

Removed "needs-fixes" to get this into today's re-build

@melissalinkert
Copy link
Member

Re-adding "needs-fixes". I definitely don't see the test failure on develop (BIOFORMATS-trunk would also be failing if that were the case), but I do see it with develop and just this branch merged in.

Can you please double-check if ant clean jars test fails for you in both cases?

@ghost
Copy link
Author

ghost commented Nov 20, 2012

Not sure if this is related, but I'm getting failure on develop alone:

test:
    [junit] Testsuite: loci.plugins.in.AutoscaleTest
    [junit] Tests run: 1, Failures: 0, Errors: 1, Time elapsed: 0.776 sec
    [junit] 
    [junit] Testcase: testAutoscale(loci.plugins.in.AutoscaleTest): Caused an ERROR
    [junit] null
    [junit] java.lang.NullPointerException
    [junit]     at java.io.Reader.<init>(Reader.java:61)
    [junit]     at java.io.InputStreamReader.<init>(InputStreamReader.java:80)
    [junit]     at ome.scifio.common.IniParser.openTextResource(IniParser.java:169)
    [junit]     at ome.scifio.common.IniParser.parseINI(IniParser.java:99)
    [junit]     at loci.common.IniParser.parseINI(IniParser.java:97)
    [junit]     at loci.plugins.prefs.OptionsList.<init>(OptionsList.java:66)
    [junit]     at loci.plugins.in.ImporterOptions.<init>(ImporterOptions.java:158)
    [junit]     at loci.plugins.in.AutoscaleTest.setup(AutoscaleTest.java:247)
    [junit]     at loci.plugins.in.AutoscaleTest.test(AutoscaleTest.java:212)
    [junit]     at loci.plugins.in.AutoscaleTest.test(AutoscaleTest.java:171)
    [junit]     at loci.plugins.in.AutoscaleTest.testAutoscale(AutoscaleTest.java:99)
    [junit]     at net.sf.antcontrib.logic.IfTask.execute(IfTask.java:197)

@melissalinkert
Copy link
Member

That definitely shouldn't be happening, but is unlikely to be related to this PR at all as it indicates that importer-options.txt in components/loci-plugins/src/loci/plugins/in/ is missing. Were there any missing files or unstaged changes in your checkout when you saw this?

@ghost
Copy link
Author

ghost commented Nov 21, 2012

All testsuite failures fixed for me now.

melissalinkert added a commit that referenced this pull request Nov 27, 2012
CoreMetadata: Simplify copying and cloning
@melissalinkert melissalinkert merged commit 5cc36bc into ome:develop Nov 27, 2012
hflynn pushed a commit to hflynn/bioformats that referenced this pull request Oct 11, 2013
Make the copyright date the current year
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants