-
Notifications
You must be signed in to change notification settings - Fork 247
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
Flex: Merge IDR changes and add new group plate option #3537
Conversation
Swapped the default behaviour due to large number of repo test failures: https://merge-ci.openmicroscopy.org/jenkins/job/BIOFORMATS-test-folder/55446/console |
Moving to 6.6.0 as this PR is memo breaking and not suitable for a patch release |
Initial testing using a copy of this plate from idr0001 with an options file setting
I will keep investigating tomorrow. |
Performed some additional testing using the first plate acquisition of the sample idr0001 fileset with the command-line tools with and without the Without the option, In both cases, the initial steps of the reader initialization parses all flex files across measurements. Looking more closely at the diff between this PR and the state of the reader on https://github.com/IDR/bioformats diff --git a/components/formats-gpl/src/loci/formats/in/FlexReader.java b/components/formats-gpl/src/loci/formats/in/FlexReader.java
index f2beb13f26..9a50150ee2 100644
--- a/components/formats-gpl/src/loci/formats/in/FlexReader.java
+++ b/components/formats-gpl/src/loci/formats/in/FlexReader.java
@@ -99,6 +99,9 @@ public class FlexReader extends FormatReader {
// -- Fields --
+ public static final String GROUP_PLATES = "flex.group.plate";
+ public static final boolean GROUP_PLATES_DEFAULT = false;
+
/** Camera binning values. */
private int binX, binY;
@@ -269,10 +272,11 @@ public class FlexReader extends FormatReader {
int nBytes = 0;
int bpp = 0;
double factor = 0;
+
try (RandomAccessInputStream s =
new RandomAccessInputStream(getFileHandle(file.file))) {
- IFD ifd;
TiffParser tp = new TiffParser(s);
+ IFD ifd;
if (file.offsets == null) {
if (imageNumber < file.ifds.size()) {
ifd = file.ifds.get(imageNumber);
@@ -305,10 +309,13 @@ public class FlexReader extends FormatReader {
offsets[i] += offset;
}
ifd.putIFDValue(tag, offsets);
- } else if (file.offsets != null) {
+ }
+ else if (file.offsets != null) {
ifd = tp.getIFD(file.offsets[imageNumber]);
}
else {
+ tp.getStream().close();
+ s.close();
return buf;
}
nBytes = ifd.getBitsPerSample()[0] / 8;
@@ -386,6 +393,25 @@ public class FlexReader extends FormatReader {
measurementFiles = new ArrayList<String>();
acquisitionDates = new HashMap<Integer, Timestamp>();
+ Location currentFile = new Location(id).getAbsoluteFile();
+ Location dir = currentFile.getParentFile();
+ runDirs = new ArrayList<Location>();
+ if (!dir.getName().startsWith("Meas_") || !groupPlates()) {
+ runDirs.add(dir);
+ }
+ else {
+ // look for other acquisitions of the same plate
+ dir = dir.getParentFile();
+ String[] parentDirs = dir.list(true);
+ Arrays.sort(parentDirs);
+ for (String d : parentDirs) {
+ Location f = new Location(dir.getAbsoluteFile(), d);
+ if (f.isDirectory() && d.startsWith("Meas_")) {
+ runDirs.add(f);
+ }
+ }
+ }
+
if (checkSuffix(id, FLEX_SUFFIX)) {
initFlexFile(id);
}
@@ -394,6 +420,7 @@ public class FlexReader extends FormatReader {
}
else initMeaFile(id);
+ if (runCount == 0) runCount = 1;
if (plateCount == flexFiles.size() / runCount) {
plateCount /= wellCount;
if ((plateCount % fieldCount) == 0) {
@@ -531,24 +558,6 @@ public class FlexReader extends FormatReader {
if (doGrouping) {
// group together .flex files that are in the same directory
- Location dir = currentFile.getParentFile();
- runDirs = new ArrayList<Location>();
- if (!dir.getName().startsWith("Meas_")) {
- runDirs.add(dir);
- }
- else {
- // look for other acquisitions of the same plate
- dir = dir.getParentFile();
- String[] parentDirs = dir.list(true);
- Arrays.sort(parentDirs);
- for (String d : parentDirs) {
- Location f = new Location(dir.getAbsoluteFile(), d);
- if (f.isDirectory() && d.startsWith("Meas_")) {
- runDirs.add(f);
- }
- }
- }
-
for (Location runDir : runDirs) {
String[] files = runDir.list(true);
@@ -557,7 +566,7 @@ public class FlexReader extends FormatReader {
LOGGER.debug("Checking if {} belongs in the same dataset.", file);
if (file.endsWith(".flex") && file.length() == 14) {
flex.add(new Location(runDir, file).getAbsolutePath());
- LOGGER.debug("Added {} to dataset.", flex.get(flex.size() - 1));
+ //LOGGER.debug("Added {} to dataset.", flex.get(flex.size() - 1));
}
}
}
@@ -576,8 +585,8 @@ public class FlexReader extends FormatReader {
LOGGER.debug("Determined that {} .flex files belong together.",
files.length);
- runCount = runDirs.size();
-
+ runCount = 1;
+ if (runCount == 0) runCount = 1;
groupFiles(files, store);
populateMetadataStore(store);
}
@@ -661,6 +670,7 @@ public class FlexReader extends FormatReader {
store.setWellSampleIndex(new NonNegativeInteger(i), pos[2], well, sampleIndex);
store.setWellSampleImageRef(imageID, pos[2], well, sampleIndex);
+ if (runCount == 0) runCount = 1;
store.setPlateAcquisitionWellSampleRef(wellSample, 0, pos[3], i % (getSeriesCount() / runCount));
}
@@ -1325,10 +1335,8 @@ public class FlexReader extends FormatReader {
if (firstBarcode == null) {
firstBarcode = barcode;
}
-
if (compressed == firstCompressed && barcode.equals(firstBarcode) &&
- ifdCount <= firstIFDCount)
- {
+ ifdCount == firstIFDCount) {
int[] well = getWell(file);
int field = getField(file);
if (well[0] > nRows) nRows = well[0];
@@ -1394,14 +1402,21 @@ public class FlexReader extends FormatReader {
for (String f : sortedFiles) {
files.add(f);
}
+ if (runCount == 0) runCount = 1;
for (int field=0; field<nFiles; field++) {
FlexFile file = new FlexFile();
file.row = row;
file.column = col;
+ LOGGER.info("FlexTest: RunCount = " + runCount + " NFiles = " + nFiles);
+ if (runCount == 0) runCount = 1;
+ if (nFiles == 0) nFiles = 1;
+ if (runCount > nFiles) {
+ runCount = 1;
+ }
file.field = field % (nFiles / runCount);
file.file = files.get(field);
- file.acquisition = runDirs.size() == 0 ? 0:
+ file.acquisition = (runDirs == null || runDirs.size() == 0) ? 0:
runDirs.indexOf(new Location(file.file).getParentFile());
if (file.file == null) {
@@ -1846,6 +1861,9 @@ public class FlexReader extends FormatReader {
if (y != null) binY = Integer.parseInt(y);
}
}
+ else if (qName.equals("Plate")) {
+ plateCount++;
+ }
else if (qName.equals("WellCoordinate")) {
if (wellNumber.length == 1) {
wellNumber[0][0] = Integer.parseInt(attributes.getValue("Row")) - 1;
@@ -2137,4 +2155,12 @@ public class FlexReader extends FormatReader {
}
}
+ public boolean groupPlates() {
+ MetadataOptions options = getMetadataOptions();
+ if (options instanceof DynamicMetadataOptions) {
+ return ((DynamicMetadataOptions) options).getBoolean(
+ GROUP_PLATES, GROUP_PLATES_DEFAULT);
+ }
+ return GROUP_PLATES_DEFAULT;
+ }
} There are a few places hardcoding |
With the last changes, the non-regression tests are successfully passing with the default option behavior esp. using the representative plate from idr0001. Turning the plate grouping option on either via passing it in the command line or using an options file with a copy of the fileset leads to a NullPointerException though
|
Conflicting PR. Removed from build BIOFORMATS-push#1070. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#7. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#8. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#9. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#4. See the console output for more details.
--conflicts |
Conflicting PR. Removed from build BIOFORMATS-push#1120. See the console output for more details.
|
Closing this as we might be exploring other options than trying to backport the IDR specific changes to the mainline. Effectively, this feature has only ever been used in the context of one studies ( |
Reopening for investigation following discussion at IDR meeting |
Testing on
Restarted server Viewing images from first acquisition of first plate from idr0001 worked OK (after short wait for memo file to be created).
Relevant memo file logs, creation loading etc
|
Excluding due to failures in jdk17 tests when saving memo files: https://merge-ci.openmicroscopy.org/jenkins/job/BIOFORMATS-test-folder/47958/console |
I was reminded of this when cleaning up old tabs I have open.... |
Since this PR is getting merged into builds now (e.g. today's https://merge-ci.openmicroscopy.org/jenkins/job/BIOFORMATS-push/45/console) tried testing again... Testing on As omero-server
Restarted server... Tried viewing Preview panel of first Well from 2nd Acquisition of the first idr0001 Plate...
|
As discussed this morning, we need to be sure that the memo file is deleted, and we're not using a memo file generated before the fix was applied... First plate:
Tried viewing image from 3rd Acquisition of first plate
|
Excluding until the warning PR's have all been merged |
Closing this as the IDR team have opted for the approach of converting the affected dataset to NGFF, as such this PR is no longer required. |
This PR attempts to merge the IDR specific Flex changes from https://github.com/IDR/bioformats/commits/master/components/formats-gpl/src/loci/formats/in/FlexReader.java
It also adds a new option flex.
group.plate
which can be used to configure the IDR behaviour. Based on initial feedback I have defaulted the option to be on.This will require a minor release and will be memo file breaking.