-
Notifications
You must be signed in to change notification settings - Fork 3
Fix file reading errors #64
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
base: main
Are you sure you want to change the base?
Fix file reading errors #64
Conversation
* update labels parameter to include default * [pre-commit.ci lite] apply automatic fixes * Enable Linux compatibility with dependency changes and improved dev tooling (WayScience#17) * run linux tests on dev branch * don't show if headless * install qt deps * try async setting * Update run-tests.yml * try additional qt libs * troubleshoot further with docker * linting * Update run-tests.yml * Update run-tests.yml * Update run-tests.yml * Update uv.lock * fallback to napari[all] and setup qt for ubuntu * move to macos-14 for tests * add python3-pyqt5 installation * attempt to install different qt deps * Update run-tests.yml * attempt another linux dep step * update for linting step * update docs * [pre-commit.ci lite] apply automatic fixes * fix conflict i hope * [pre-commit.ci lite] apply automatic fixes --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Dave Bunten <dave.bunten@cuanschutz.edu>
* add synapse download enhancement to test real data * Update conftest.py * Update coverage-badge.svg * [pre-commit.ci lite] apply automatic fixes * assert that the files exist prior to tests * Update conftest.py * Update conftest.py * [pre-commit.ci lite] apply automatic fixes * track down missing files within gh actions env * [pre-commit.ci lite] apply automatic fixes * pathlib correction * check for the token * [pre-commit.ci lite] apply automatic fixes * retry * move to pull request target * readd pull request trigger * remove verbose pytest output * linting * [pre-commit.ci lite] apply automatic fixes * Apply suggestions from code review Co-authored-by: Gregory Way <gregory.way@gmail.com> * [pre-commit.ci lite] apply automatic fixes --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Gregory Way <gregory.way@gmail.com>
…ce#35) * enable downstream branches to be checked out * Update .pre-commit-config.yaml * linting
* add nviz cli * [pre-commit.ci lite] apply automatic fixes * updates based on feedback Co-Authored-By: Mike Lippincott <58147848+MikeLippincott@users.noreply.github.com> * ignore testing utils * ignore conftest for coverage --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Mike Lippincott <58147848+MikeLippincott@users.noreply.github.com>
…e#40) * move to macos for pre-commit checks * linting * add context
…enges (WayScience#42) * Create check.py * path report capabilities * linting * [pre-commit.ci lite] apply automatic fixes * remove pull_request set of testion actions * Update .pre-commit-config.yaml * Update .pre-commit-config.yaml * use pull request src instead of target for chkout * add debug for actions * remove debug * update to avoid coverage discrepancies * Update coverage-badge.svg * move to macos for pre-commit checks * linting * ignore tests for coverage * remove coverage reporting * remove shellcheck because of docker * add path reporter to cli * update for return code expectations * add complete test data * update for tests and readd coverage just in case * test locally * Update run-tests.yml * Update coverage-badge.svg * Update coverage-badge.svg * readd coverage Co-Authored-By: Jenna Tomkinson <107513215+jenna-tomkinson@users.noreply.github.com> * Update .pre-commit-config.yaml --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Jenna Tomkinson <107513215+jenna-tomkinson@users.noreply.github.com>
* update dockerfile for uv * update deps and fix pre-commit config * update dockerfile and poe task
* fix docs builds with linux headless config * Update .pre-commit-config.yaml * apply changes for pre-commit * Update coverage-badge.svg
* add software doi for reference * Update .pre-commit-config.yaml
* update pre-commit-lite conditions * correction
* update gh actions macos runner * Update .pre-commit-config.yaml
* Add Almanack to pre-commit checks * Update .pre-commit-config.yaml * add the fork remote for upstream ref * debugging information for git * Update run-tests.yml * remove git block
if not headless: | ||
# Start the Napari event loop | ||
napari.run() | ||
napari.run(force=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a special note here about why we use force=True
(I'm guessing it's to see that Napari loads but might sometimes not have an image to display).
assert tif.shape > 3 # Ensure there are multiple dimensions | ||
assert tif.shape == 4 # includes channel names :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that tests were failing after resolving an issue with the uv.lock
file. Consider reducing this to one line (the explicit check for a len of 4 in a tuple returned from shape).
assert tif.shape > 3 # Ensure there are multiple dimensions | |
assert tif.shape == 4 # includes channel names :) | |
# Ensure tif has multiple dimensions | |
# and includes channel names :) | |
assert len(tif.series[0].shape) == 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file looks to be corrupted. Consider removing it and re-running uv lock
to recreate it.
|
||
from .image_meta import extract_z_slice_number_from_filename, generate_ome_xml | ||
|
||
def read_zstack_image(tiff_file_path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding tests for this new function which otherwise is only implicitly checked.
img = np.squeeze(img) | ||
|
||
if img.dtype != np.uint16: | ||
if img.dtype in [np.float32, np.float64]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using the following to check for all floats.
if img.dtype in [np.float32, np.float64]: | |
if np.issubdtype(img.dtype, np.floating): |
if img.dtype != np.uint16: | ||
if img.dtype in [np.float32, np.float64]: | ||
# For float images, first rescale to 0-1 range, then convert | ||
img = skimage.exposure.rescale_intensity(img, out_range=(0, 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you feel this is important enough to warn the user about? If so, consider adding a log message for this. It could be that we document this behavior upfront (perhaps in the docstring of this function) and leave as-is for now.
Also, @MikeLippincott, note that within the PR description you can use keywords to close PR's or issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job fixing the lockfile! I noticed the new error from CI relates to PyQt. One workaround might be to change the napari[all]
dependency to use napari[pyside2]
or napari[pyside2, optional]
. Another option might be to constrain PyQt versions (earlier versions appeared to work with MacOS arm-based archs).
This PR adds a way to format input images (ome-tiff or tiff) to be compatible with nviz functions. Changes such as retyping, and dimensionality squeezing.
This PR is somewhat related to PR #12 if this one is merged #12 will be closed.
What kind of change(s) are included?
Checklist