Closed anntzer closed 1 year ago
Base: 61.64% // Head: 77.96% // Increases project coverage by +16.31%
:tada:
Coverage data is based on head (
f525608
) compared to base (5ec734d
). Patch coverage: 81.63% of modified lines in pull request are covered.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Wow!!! What a gift for pims! Thanks @anntzer !!
I see no reason not to merge now, especially since this PR has had some time to season, and because we really could use working CI again.
I see that you are in support of some further changes, which I summarize as
pylibtiff
moviepy
I see no problem with any of those given how much the package ecosystem has evolved since pims was first written, but I agree that those should be done in separate PRs since they would require some careful changes to the code, and of course there could be some users out there for whom these would be big problems. We already have #426 to discuss the bioformats change; should we open additional issues to discuss the first two?
I would also drop imageio-ffmpeg (per https://imageio.readthedocs.io/en/v2.25.1/_autosummary/imageio.plugins.ffmpeg.html#module-imageio.plugins.ffmpeg).
While I appreciate that this may be disruptive for some users, I simply notice that maintaining working CI for pims seems to have been challenging recently, and e.g. in the PR above I had to make various fixes for a number of readers, but I have no idea of whether the pylibtiff reader works as even installing that package properly is challenging (and I had limited interest in fixing that when the package README itself suggests that other tiff libraries may be better suited). Also note that my interest in getting CI fixed is actually to help the review of my other PRs (right now only #434; I could perhaps also give a try at fixing #436 now that CI is fixed).
You can open separate issues to keep track of the decision process but I don't really have much to add on top of that.
I went for the big omnibus PR...
Unless there's strong reasons to do otherwise, I would also suggest dropping support for pylibtiff (and perhaps also just pick a side between pillow's tiff and tifffile); and likewise pick one movie reader (pyav seems favored by skimage https://scikit-image.org/docs/stable/user_guide/video.html#pyav, and has wheels so installation should be easy in practice).