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

Python: use requirements.txt from dependencies #493

Open
KrisThielemans opened this issue Feb 15, 2021 · 13 comments
Open

Python: use requirements.txt from dependencies #493

KrisThielemans opened this issue Feb 15, 2021 · 13 comments

Comments

@KrisThielemans
Copy link
Member

Since SyneRBI/SIRF#870, SIRF has requirements.txt and ci-requirements.txt. We should use this in the SuperBuild, and also for any other projects that have such files, or where we know what they are.

I'm not sure how to handle this, but what about making on-the-fly one requirements.txt from all the existing ones (and any additions) and install that as env_sirf_requirements.txt etc? Might be tricky once version numbers get added in there...

@KrisThielemans
Copy link
Member Author

@casperdcl do you have any suggestions on how this is usually handled?

@casperdcl
Copy link
Member

Er... why would you pull in the deps requirements? The whole point of pip install dep is that dep's own requirements automatically get installed.

SIRF need only define its direct dependencies.

@casperdcl
Copy link
Member

@KrisThielemans
Copy link
Member Author

yes, the latter. However, the complication I see is that the VM/docker/superbuild also installs other things than SIRF, which might have their own requirements. Example: SIRF-exercises needs (or could use) numba etc. So, arguable the SuperBuild should (allow the user to) install Python dependencies from all the packages.

Maybe this would all disappear if we'd be able to do a pip install SIRF etc

@casperdcl
Copy link
Member

Hmm yes, quite. The entire SuperBuild really could disappear if we could pip install sirf. The "correct" way would have pip automatically download all build dependencies (including cmake) and build everything in a cross-plaform compatible way. I don't have time to set that up (not for free at least :P).

In the meantime, we can remove bits of the requirements.txt from this SuperBuild and pip install -r SIRF/requirements.txt instead (after the clone step).

@KrisThielemans
Copy link
Member Author

yes. This is getting close to our usual confusion. The SIRF-SuperBuild does (or can do) quite a lot more than building SIRF and its dependencies. So pip install sirf or conda install sirf would use SIRF/requirements.txt, but as I stated, the SuperBuild requirements can be larger.

In the meantime, we can remove bits of the requirements.txt from this SuperBuild and pip install -r SIRF/requirements.txt instead (after the clone step).

Tricky bit is that the SIRF clone only happens after the make. Hence my suggestion to let the Superbuild a "super-requirements.txt", but maybe I'm mis-using this.

@ashgillman
Copy link
Member

I think we're on the same page, but to be explicit: I think confusion/complication here is between the requirements.txt, which is designed to define an environment or runtime, or part thereof, and the definition of dependencies, which should really belong in a setup.py. I've mentioned it super briefly in SyneRBI/SIRF#870.

Obviously, dependencies should be transitive, but the requirements.txt isn't really designed that way. You can, as mentioned, effectively "union" environments by using multiple requirements.txt, assuming they don't conflict.
I think what you are hoping for Kris might be better served with dependencies rather than combining environments. This doesn't have to go as far as a pip install sirf though.

I need to look into this a bit more tomorrow though to have a full understanding. For example, I'm not sure when or if SuperBuild handles Python dependencies, but if it could be at the end it wouldn't matter if we have to wait for the sources of each dependency to be pulled, I guess you could grep for a */*requirements.txt.

The "correct" way would have pip automatically download all build dependencies (including cmake) and build everything in a cross-plaform compatible way.

That's interesting, I always assume the "correct" way would still for CMake to be the installer, and have it run the setup.py. I looked into this a few years ago, but might have been barking up the wrong tree if this is the case.
@casperdcl are you aware (I'm sure they exist, but a good one) of a repo using CMake (better, SuperBuild) + setup.py that would make a good template project

@KrisThielemans
Copy link
Member Author

No, the Superbuild doesn't currently install any Python requirements nor tracks them.

By the way, conda feedstocks maintain their own list of requirements of course (not via requirements.txt). Probably unavoidable.

@casperdcl
Copy link
Member

For clarification, the gold standard would be:

  1. leave it to the user to install
    • python (can be installed via conda)
    • g++/visual studio (g++ can be installed via conda)
    • (optional) CUDA drivers & toolkit (toolkit can be installed via conda)
    • (optional) matlab (free runtime may be installable via pip soon)
  2. one command to install SIRF
    • pip install sirf # builds & installs basic requirements
    • pip install sirf[gpu,some_other_option,examples] # also builds & installs GPU projector, example subfolder, etc

I've done this for other projects but it would take me a few weeks of full time effort to sort it out for SIRF (pip is capable of downloading and running cmake in a temporary build directory and installing things itself correctly based on cmake's build output. I wouldn't be too surprised if in the next year it gains the ability to download compiler toolchains too).

There aren't any good template repos I can recommend. The lack of free guidance is frankly due to complexity. People who develop tools to solve this problem don't tend to also provide docs/templates. I've fixed a few bugs in some of said tools, but none of them are documented properly and all of the example/template repos are outdated/broken.

Take the SuperBuild itself, for example: even the documentation for how to use it is a minefield - let alone documentation for how to develop it :)

@ashgillman
Copy link
Member

Hmm, yes that seems rather confusing in the SIRF context. Once you forfeit CMake running the show, then I don't see how the SuperBuild is able to orchestrate making sure libraries align between the various subprojects, and I doubt pip is able to tweak installation options in the way CMake can (e.g., using a specific STIR branch). OTOH, all this hidden from an end-user sounds nice, and that workflow is super simple to anyone using Python.

matlab (free runtime may be installable via pip soon)

wow

@ashgillman
Copy link
Member

I've had a quick search for examples to no avail - SimpleITK and Paraview are two C++/CMake/CMake SuperBuild projects with Python wrappings I know of. But it seems neither are really doing dependency wrapping, although both seem to restrict dependencies to a mimimum.
https://github.com/SimpleITK/SimpleITK/search?q=numpy

SimpleElastix merges its setup code from SimpleITK.

Anyway, an alternative I would see would be declaring the SIRF Python dependencies in the setup.py.in and having them be installed with SIRF's CMake build calls the Python installer. This doesn't get us to pip install sirf, but it does mean that the CMake build and hence SuperBuild will install the dependencies. The only issues is that I'm not sure, but these dependencies might get installed into the SIRF build dir - probably this would be most desirable though. If not, then its not clear where they should be installed - both user and system installations would be surprising, a CMake Build should be self-enclosed.

@ashgillman
Copy link
Member

Actually, that's a question @KrisThielemans . If the SuperBuild did install Python dependencies - where would you want them installed? Logical to me would be the SuperBuild build dir, and then to CMAKE_INSTALL_DIR (e.g. /usr/local) if you run cmake install.

@ashgillman
Copy link
Member

Or, from the original description, do you just envisage the SuperBuild spitting out a requirements.txt amalgamated from sub-projects, without installing anything, and then the user can do what they will with that?

Sounds like this? pypa/pip#53
They currently recommend: https://github.com/jazzband/pip-tools/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants