DeltavisionReader.java: complete objectives metadata from softWoRx 7.0.0 #3204
Conversation
| case 10116: // Olympus 4X/0.16, U-Plan S-Apo, UIS2, 1-U2B822 | ||
| lensNA = 0.16; | ||
| magnification = 4.0; | ||
| workingDistance = 13; |
There was a problem hiding this comment.
This fails Travis with
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.7.0:compile (default-compile) on project formats-gpl: Compilation failure
[ERROR] /home/travis/build/openmicroscopy/bioformats/components/formats-gpl/src/loci/formats/in/DeltavisionReader.java:[1573,27] incompatible types: int cannot be converted to java.lang.Double
[ERROR] -> [Help 1]
Can you try workingDistance = 13.0; ?
|
Sorry. I should have tried to rebuild it myself first. It is fixed now (at least it builds) For what is worth, the whole switch/case blocks were generated automatically with this perl script https://github.com/MicronOxford/scripts/blob/master/auto/dvlenses2readercases.pl The script has an issue with figuring out the right correction type. It tries a series of regexs but they are not always in the same order so sometimes picks UV and other times PlanApo. I could fix it by forcing a order on the regexp but I think the issue here is that I don't even know myself which correction to use sometimes. For example, for "Olympus 100X/1.30, D Plan Apo UV" should it be PlanApo or UV? |
There was a problem hiding this comment.
Overall this is a very impressive and exhaustive parsing of the metadata available from the proprietary software.
Most of the changes are effectively metadata additions. The only notable exceptions being:
- a few value updates like the
workingDistanceof10412which all seem to be in agreement with the objective description calibratedMagnificationswitched tomagnificationas discussed in the PR description. Given the semantic difference between nominal magnification and calibrated magnification, I agree anything that is determined from thelensIDshould be treated as nominal magnification. Unless the calibrated magnification is expected to be stored elsewhere, a follow-up might involve the cleanup of thecalibratedMagnificationhandling.
With this PR included the repository tests against our Deltavision sample files keep passing - see https://web-proxy.openmicroscopy.org/west-ci/job/BIOFORMATS-test-folder/25363/. Importing our these datasets into a staging server with a development version of Bio-Formats including this PR shows the Acquisition metadata is properly populated e.g. for 1-U2B825 which was not previously handled:
Barring one inline question about two model values, this should be a great addition in the upcoming 5.9.2 release of Bio-Formats.
| immersion = getImmersion("Oil"); | ||
| model = "44 07 51 "; |
There was a problem hiding this comment.
This objective and the following one have trailing whitespaces in the model name. Is that expected?
There was a problem hiding this comment.
Not really. I should have trimmed the values when I parsed it from the data files we get from softworx. Now fixed in fdc0a27
With commit 735eace I removed the whole |
Following an unrelated discussion on issue #2656 , I took a look at the list of objective ids for missing values. This commit completes the list with the data from the latest version of softWoRx (version 7.0.0).
This commit also adds the name of the objective in front of the id as a comment which hopefully helps in finding issues in filling the correction type. I was also confused about the calibratedmagnification, I'm not sure how to know if the magnification value is calibrated or not from the data on that file.