-
Notifications
You must be signed in to change notification settings - Fork 251
CoreMetadata: Simplify copying and cloning #215
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
CoreMetadata: Simplify copying and cloning #215
Conversation
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.
|
Fails tests for jpeg2000 and some related formats, so will need some further work. |
|
All tests passing now; other than a single pyramid failure picked up from the leicascn-newschema code. The earlier failure was a missbuild. |
|
Note that the failure here is the rounding issue we discussed last week. |
|
OMERO-merge-develop job 45 failed with: `` copy-source: compile: Marking broken. |
|
Removing from builds until that is resolved. |
|
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). |
|
Removed "needs-fixes" to get this into today's re-build |
|
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 |
|
Not sure if this is related, but I'm getting failure on develop alone: |
|
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? |
|
All testsuite failures fixed for me now. |
CoreMetadata: Simplify copying and cloning
Make the copyright date the current year
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.