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

bugfix/squeeze-extra-dims #32

Merged
merged 2 commits into from
Sep 25, 2021
Merged

bugfix/squeeze-extra-dims #32

merged 2 commits into from
Sep 25, 2021

Conversation

evamaxfield
Copy link
Collaborator

Pull request recommendations:

  • Name your pull request your-development-type/short-description. Ex: feature/read-tiff-files
  • Link to any relevant issue in the PR description. Ex: Resolves [admin/build-py39 #12], adds tiff file format support

Resolves #24

  • Provide context of changes.

Simple fix. xarray provides a squeeze function that also squeezes the dims out too

Tagging @psobolewskiPhD if you are happy with the shape changes reported in the tests please let me know. Will merge and release a new version on Monday.

  • Provide relevant tests for your feature or bug fix.
  • Provide or update documentation for any feature added by your pull request.

Thanks for contributing!

@evamaxfield evamaxfield added the bug Something isn't working label Sep 24, 2021
@evamaxfield evamaxfield self-assigned this Sep 24, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2021

Codecov Report

Merging #32 (1b7785c) into main (0e888bc) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #32   +/-   ##
=======================================
  Coverage   83.05%   83.05%           
=======================================
  Files           5        5           
  Lines         177      177           
=======================================
  Hits          147      147           
  Misses         30       30           
Impacted Files Coverage Δ
napari_aicsimageio/tests/test_core.py 100.00% <ø> (ø)
napari_aicsimageio/core.py 85.45% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e888bc...1b7785c. Read the comment docs.

@psobolewskiPhD
Copy link
Collaborator

Awesome!
For the regular TIFF: s_1_t_1_c_1_z_1.tiff I get:

AICSImageIO: Reader will load image in-memory: False
10:19:20 ERROR Failed to parse XML for the provided file.
10:19:20 ERROR not well-formed (invalid token): line 1, column 6
10:19:20 ERROR Failed to parse XML for the provided file.
10:19:20 ERROR not well-formed (invalid token): line 1, column 6
AICSImageIO: Reader will load image in-memory: False

But I think it opened properly.
But the CZI and LIF (s_1_t_1_c_2_z_1.lif) files work great!

@evamaxfield
Copy link
Collaborator Author

Awesome!
For the regular TIFF: s_1_t_1_c_1_z_1.tiff I get:

AICSImageIO: Reader will load image in-memory: False
10:19:20 ERROR Failed to parse XML for the provided file.
10:19:20 ERROR not well-formed (invalid token): line 1, column 6
10:19:20 ERROR Failed to parse XML for the provided file.
10:19:20 ERROR not well-formed (invalid token): line 1, column 6
AICSImageIO: Reader will load image in-memory: False

But I think it opened properly.
But the CZI and LIF (s_1_t_1_c_2_z_1.lif) files work great!

Yep. It is simply running through the readers. It tries the OME-TIFF reader and fails but then rolls back to the TIFF reader. No worries.

@evamaxfield evamaxfield merged commit 7605974 into main Sep 25, 2021
@evamaxfield evamaxfield deleted the bugfix/squeeze-extra-dims branch September 25, 2021 17:29
@psobolewskiPhD
Copy link
Collaborator

psobolewskiPhD commented Feb 19, 2022

Not to nag, but...any idea when this fix may end up in a release?
❤️

@evamaxfield
Copy link
Collaborator Author

If blocking I can release a new patch right now but likely next week.

The plan of work is as follows:

  1. Remove CZI install pattern from base aicsimageio: admin/remove-czi-install-pattern aicsimageio#376
  2. Release 4.6.0 version of aicsimageio
  3. Update this very old PR to use aicsimageio>=4.6.0 and incorporate the above install pattern changes / dep changes

So really just waiting on eyes on 1, the CZI install pattern changes. But now that I look at this, nothing is directly holding it up. I could make and release the dep changes now.

I will do so now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty dims when importing TIFF, LIF (vs say CZI)
3 participants