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
COMP: Update python packages to latest #7285
COMP: Update python packages to latest #7285
Conversation
The tests on Windows are equivalent to the current Preview release test report on Windows (https://slicer.cdash.org/viewTest.php?onlyfailed&buildid=3178343) which has 8 failures. |
python-dateutil and typing-extensions are new dependencies of PyGithub
0b32c7b
to
8c2ac0b
Compare
I will start a clean build locally and then will move forward with approval and integration. The nightly build will allow to confirm a clean build effectively work on the supported platforms. |
FYI, Pillow==10.1.0 caused a bit of trouble in TotalSegmentator. This new minor version of Pillow has been released just two weeks ago. Many Python packages reject yet untested minor versions by default (have requirement now for pilllow<10.1 until they have tested that their package actually works with 10.1). Therefore, if I install almost any scientific Python package then pillow will be downgraded to 10.0. However, if something has already imported pillow then this downgrade fails (file access denied). While we could think about making downgrades/upgrades more robust (e.g., somehow allow extensions to run pip when Slicer application is not running; or check if we can use venv in Slicer) |
I believe in this case scikit-image usually doesn’t have an upper pin. They added the upper pin for pillow only after observing an issue related to imageio. Therefore their approach is not proactive in always having an upper pin where they don’t bump until they have tested that their package actually works with new versions of dependencies. Instead they are reactive and set upper limitations only if they have found some issue. From the quarterly update of python packages to latest for well over a year, the top packages including scientific packages have worked well with each other. I think issues arise with the packages with a ton of dependencies like TotalSegmentor which uses some smaller packages which may not have the most resources or in the dependency chain packages have questionable version requirements. To handle this situation I think venv usage in Slicer is really going to be the only reasonable way to make sure 3rd party packages can install their correct versions that may be incompatible with Slicer core versions that have already been imported. @lassoan Did automated SlicerTotalSegmentor test cases on cDash show that installation failed due to the permission error? |
I think SlicerTotalSegmentor also has the ability to package specific Python versions at build time rather than installing at runtime. Some discussion of strict version requirements being part of #7171. |
Prohibiting new minor release might not be a general policy but I see it frequently enough that it is something to plan for. The fact is that it may take 1-2 months for a new major.minor release to be tests and stabilized, so we should define some embargo period for them. We can assume patch releases are fine and we can install them immediately. To be precise, TotalSegmentator only has one significant dependency: nnunet. Nnunet is the one that has lots of other dependencies. There are several other extensions that use nnunet, which is probably the most successful image segmentation model, so it deserves attention (make sure it can be easily installed). Bundling Python packages with an extension is good for taking care of missing wheels, but I don't see how it can resolve incompatibities. It would be great if you could test if venv works within Slicer. In most of the deep learning processing modules it is fine if we run the computation in a separate process. If that process could run in a separate Python environment then we would have less dependency conflicts to worry about. We would probably still want to install huge packages, such as pytorch, in a shared environment, but incompatibilities due to SimpleITK and other random issues between small packages could be taken care of. We don't run any automated tests for totalsegmentator, as we don't have CI on a GPU-equipped computer (although it could be set up). The main problem is probably testing extensions that rely on other extensions and that installing Python packages can mess up the Python environment for other purposes extensions. |
Yes it would be nice to setup automated testing within the individual extension repo using a build tree when needed. For SlicerTotalSegmentor being Python based running tests by downloading latest Slicer version and running the self tests should be fairly easy to implement. Then the testing would be localized (not impacting other Slicer extension builds) and results reported in the repo github actions. The purpose of the testing to make sure SlicerTotalSegmentor is always working if it is one of the most commonly used extensions. |
Python packages were last updated on July 15th 2023 (7d58308). This PR updates python packages to their latest versions. This type of update is something I usually do on about a quarterly cadence (every 3 months).
Note that SimpleITK isn't being updated here as we build it from source rather than using the provided whl on pypi.
Regarding the update of
numpy
from 1.25.1 to 1.26.1, the 1.26.x series is a continuation of the 1.25.x series with the major change being a new build system for the package. See https://numpy.org/doc/stable/release/1.26.0-notes.htmlTesting Results:
Windows