Skip to content
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

SVS: update isThisType to reject files with a single IFD #4202

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

melissalinkert
Copy link
Member

See IDR/idr-metadata#699.

With this change, I would expect the files from idr0008 to be detected as "plain" TIFF; showinf or similar should initialize and show the image without error. I tested with an artificial file created by:

$ bfconvert "test&sizeX=1000&sizeY=1000.fake" svs-test.tif
$ cat comment.txt 
Aperio Image Library v8.2.43
81713x37331 [70791,733 2942x2942] (0x0) RAW;Scan
{composite image: 41 files}|AppMag = 20
|StripeWidth = 2032
|ScanScope ID = SS1644
|Filename = 156866
|Title = 
|Date = 10/10/13
|Time = 09:14:17
|User = 85966804-1588-42c3-bd9e-88ca3cfca814
|Acquisition Bit Depth = 8
|MPP = 0.4970
|OriginalWidth = 81713|Originalheight = 37331
$ tiffcomment -set comment.txt svs-test.tif

which was enough to demonstrate the original exception, but we may want one of the idr0008 files in the test repo to be safe.

As discussed separately with @sbesson, this is probably the least disruptive way to solve the problem, as it does not require modifying any of the extra image detection logic in SVSReader. I would not expect this to introduce any test failures, and it should be safe for a patch release.

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using https://ftp.ebi.ac.uk/pub/databases/IDR/idr0043-uhlen-humanproteinatlas/20210513-ftp/36576/141893_A_1_1.tif as a public representative sample of this issue, confirmed that Bio-Formats 7.3.0 fails to initialise this file with the exception reported in IDR/idr-metadata#699.

With this change included, the isThisType check is rejected for SVSReader and the file ends up being detected by the default DelegateTiffReader which matches the solution discussed with @melissalinkert.

sbesson@Sebastiens-MacBook-Pro-3 idr0043 % ~/Documents/GitHub/bioformats/tools/showinf 141893_A_1_1.tif
Checking file format [Tagged Image File Format]
Initializing reader
TiffDelegateReader initializing 141893_A_1_1.tif
Reading IFDs
Populating metadata
Checking comment style
Populating OME metadata
Initialization took 0.099s

Reading core metadata
filename = 141893_A_1_1.tif
Series count = 1
Series #0 :
	Image count = 1
	RGB = true (3) 
	Interleaved = false
	Indexed = false (false color)
	Width = 2942
	Height = 2942
	SizeZ = 1
	SizeT = 1
	SizeC = 3 (effectively 1)
	Tile size = 2942 x 118
	Thumbnail size = 128 x 128
	Endianness = intel (little)
	Dimension order = XYCZT (certain)
	Pixel type = uint8
	Valid bits per pixel = 8
	Metadata complete = true
	Thumbnail series = false
	-----
	Plane #0 <=> Z 0, C 0, T 0


Reading pixel data (0-0)
	Read 1/1 planes (100%)
[done]
0.419s elapsed (419.0ms per plane)

Launching image viewer
2024-06-27 10:10:04.813 java[72611:986981] WARNING: Secure coding is automatically enabled for restorable state! However, not on all supported macOS versions of this application. Opt-in to secure coding explicitly by implementing NSApplicationDelegate.applicationSupportsSecureRestorableState:.

Reading global metadata
BitsPerSample: 8
Comment: Aperio Image Library v8.2.43
81713x37331 [70791,733 2942x2942] (0x0) RAW;Scan

Compression: Uncompressed
ImageLength: 2942
ImageWidth: 2942
MetaDataPhotometricInterpretation: RGB
MetaMorph: no
NewSubfileType: 0
NumberOfChannels: 3
PhotometricInterpretation: RGB
PlanarConfiguration: Chunky
SamplesPerPixel: 3
{composite image: 41 files}|AppMag: 20
|Acquisition Bit Depth: 8
|Date: 10/10/13
|Filename: 156866
|MPP: 0.4970
|OriginalWidth: 81713|Originalheight = 37331
|ScanScope ID: SS1644
|StripeWidth: 2032
|Time: 09:14:17
|User: 85966804-1588-42c3-bd9e-88ca3cfca814

Reading metadata
Screenshot 2024-06-27 at 10 10 15

The nightly CI repository tests have been passing with this change including meaning that no other TIFF/SVS file in the curated QA repository are affected by this change.
As a next step, I'll add the representative TIFF file to the curated QA repository and configure it so that it's included in the nightly builds and we avoid regressions.

In the context of consuming it in an upcoming IDR deployment, keeping compatibility with the memo files generated using Bio-Formats 7.3.0 was brought up as a requirements. I also tested that a build of Bio-Formats with this PR included on top of the current HEAD of the development branch is able to load memo files created by Bio-Formats 7.3.0.
In other terms, so far no change has been merged which would cause backwards-incompatible changes in the reader serialization (like the addition of new private fields) and it should be possible to schedule a patch release of Bio-Formats 7.3.1 with a few fixes like this one which remains compatible with Bio-Formats 7.3.0 memo files.

/cc @will-moore @dgault @jburel

@dgault dgault merged commit 4860a21 into ome:develop Jul 8, 2024
18 checks passed
@melissalinkert melissalinkert deleted the svs-reject-single-ifd branch September 6, 2024 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants