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.

Sign up for GitHub

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

Jump to bottom

Feature/use bioio #376

Merged
merged 3 commits into from
Apr 19, 2024
Merged

Feature/use bioio #376

merged 3 commits into from
Apr 19, 2024

Conversation

saeliddp
Copy link
Contributor

@saeliddp saeliddp commented Apr 8, 2024 •
edited
Loading

What does this PR do?

Ports aicsimageio to bioio, which is recommended since only bioio is guaranteed to have dev support and updates moving forward. Also solves the issue we were having with bfio causing a segfault during testing. (see checks for verification). Also verified that with just the requirements in pyproject.toml, I can successfully run pytest -k segmentation.

Fixes 372

Notes

  • with bioio, you have to install plugins to deal with different image types--right now I have support for ome-tiff (non-tiled), czi, and tiff (non-globbed)
  • I imagine we need to update the requirements directory after this update since we no longer need aicsimage and now need bioio

Before submitting

  • Did you make sure title is self-explanatory and the description concisely explains the PR?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you test your PR locally with pytest command?
  • Did you run pre-commit hooks with pre-commit run -a command?

Did you have fun?

Make sure you had fun coding 🙃

@saeliddp saeliddp marked this pull request as draft April 8, 2024 18:15
@saeliddp saeliddp marked this pull request as ready for review April 8, 2024 18:15
@saeliddp saeliddp closed this Apr 8, 2024
@saeliddp saeliddp reopened this Apr 8, 2024
@saeliddp saeliddp changed the base branch from main to feature/loosen_pyproject_deps April 8, 2024 19:53
saeliddp
saeliddp commented Apr 8, 2024
View reviewed changes
cyto_dl/datamodules/czi.py
@@ -3,7 +3,7 @@

import numpy as np
import pandas as pd
from aicsimageio.aics_image import AICSImage
from bioio import BioImage
Copy link
Contributor Author

@saeliddp saeliddp Apr 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The APIs for BioImage and AICSImage are, as far as I can tell, identical. So, all we really need to do is swap out AICSImage for BioImage wherever we used it.

@hughes036
Copy link

I see the tests still failing in CI though - is that because requirements.txt still needs to be re-generated?

@saeliddp
Copy link
Contributor Author

saeliddp commented Apr 9, 2024

I see the tests still failing in CI though - is that because requirements.txt still needs to be re-generated?

Ubuntu-latest 3.8 is failing because bioio requires python >= 3.9. The windows tests have been failing since before I started working on the dependency issues; I'm not sure what the cause of that is.

@benjijamorris
Copy link
Contributor

Windows test are timing out at 70 minutes, it sounds like bioio will make tests faster + should fix this!

benjijamorris
benjijamorris requested changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@benjijamorris benjijamorris left a comment

Choose a reason for hiding this comment

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

looks fantastic! I think we're ready to support >=3.9 only

pyproject.toml Outdated
"bioio",
"bioio-czi",
"bioio-ome-tiff",
"bioio-tifffile"
]
requires-python = ">=3.8,<3.11"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this to >=3.9?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! 9d3f60

@saeliddp saeliddp requested a review from hughes036 April 9, 2024 23:00
@benjijamorris benjijamorris self-requested a review April 19, 2024 21:29
@saeliddp saeliddp merged commit 5960d84 into feature/loosen_pyproject_deps Apr 19, 2024
@saeliddp saeliddp deleted the feature/use_bioio branch April 19, 2024 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjijamorris benjijamorris benjijamorris approved these changes

@yrkim98 yrkim98 Awaiting requested review from yrkim98

@hughes036 hughes036 Awaiting requested review from hughes036

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@saeliddp @hughes036 @benjijamorris