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

Avoid hard-coded field-of-view path "0" #300

Merged
merged 4 commits into from
Sep 29, 2023

Conversation

aeisenbarth
Copy link
Contributor

@aeisenbarth aeisenbarth commented Aug 23, 2023

Fixes #248

The current code assumes image paths under a well always match field of view indices, especially that "0" exists. However, the NGFF spec allows alphanumeric paths.

This change looks up the actual image path from the first well. This should not have a performance impact since the first well's .zattrs is read anyways. It is still assumed all wells have the same first image path (which the previous code did as well). In that case, an array of zeros is read (as previously).

@will-moore
Copy link
Member

Looks good, and works for me. Is this still a [WIP]?
Don't know why the docs build failed, but it's not because of this PR

@aeisenbarth
Copy link
Contributor Author

I still wanted to look into how to cover this by tests. Maybe by parametrizing test_multiwells_plate with fields = map(str, range(3)) and fields = ["img1", "img2", "img3"].

@aeisenbarth
Copy link
Contributor Author

I added a test. Since failed image lookups fail gracefully and return zeros, I had to change the image values of the test arrays to something non-zero.

The alphanumeric test case failed on master and succeeds on this branch.

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Patch coverage is 33.33% of modified lines.

❗ Current head 4455b89 differs from pull request most recent head 1b4601d. Consider uploading reports for the commit 1b4601d to get more accurate results

Files Changed Coverage
ome_zarr/reader.py 33.33%

📢 Thoughts on this report? Let us know!.

@aeisenbarth aeisenbarth changed the title [WIP] Avoid hard-coded field-of-view path "0" Avoid hard-coded field-of-view path "0" Aug 28, 2023
@will-moore
Copy link
Member

Hi @aeisenbarth can you merge in origin/master which should fix the build failures above

@will-moore will-moore added this to the 0.8.1 milestone Sep 13, 2023
@will-moore will-moore merged commit ea18dd4 into ome:master Sep 29, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FOV names and array names are hard-coded for the HCS dataset reader
3 participants