Conversation
This introduces sets of optional-dependencies to `pyproject.toml` that allow for reduced dependency installation. The `openlifu-test-app` project, for example, can package only the hardware-communication required dependencies by using the `[io]` option. The base application as used in Slicer should now be installed with the `[app]` option.
- Trims unnecessary parts of the project, including - legacy scripts - the io module (now covered in `openlifu-sdk`) - unused parts of Database (gridweights) - Reorganizes `virtual_fit` module into `seg` - Eliminates "eager" imports in `openlifu.__init__`, to prevent the many-seconds-long importing when accessing any submodule. - Moves importing of heavier-weight packages (which are being made optional by this branch) into the methods that call them, so that the objects can be partially-used without having those dependencies imported. Code that touches `openlifu.virtual_fit` needs to be refactored to use `openlifu.seg.virtualfit`, and code that used things like `openlifu.Transducer` will need to be changed to use `openlifu.xdc.Transducer` reorganize virtual fit and geo remove gridweights Unused enhancement Update transducer.py remove legacy scripts,
|
Interesting - the CI virtual environment isn't picking up the right dependencies (because the "no-option" version grabs a super-minimal set that won't have full functionality). Not sure how to address this? I'd really like |
peterhollender
force-pushed
the
p/init_install_cleanup
branch
from
faba8a3 to
c1e3ae3
Compare
I think this is not possible, to make But that is completely fine, in my opinion. It is very reasonable to make the CI ofr example install One way to achieve something like what you are saying is to rename this package to |
| "matplotlib", | ||
| "pandas", | ||
| "scipy", | ||
| "watchdog" |
There was a problem hiding this comment.
Maybe this one should go to the cloud group rather being a core dependency
| "nvidia-ml-py" | ||
| ] | ||
| cloud = [ | ||
| "requests", |
There was a problem hiding this comment.
I think requests should be in the core depndencies group because of its use in misc things like assets.py
It is a very light dependency I think so no issue with that
| [tool.hatch.metadata] | ||
| allow-direct-references = true |
There was a problem hiding this comment.
There are no direct references so we should remvoe this. I think for anything merged to main there should not be direct references anyway, because PyPI will reject openlifu if there are
| description = "Openwater Focused Ultrasound Toolkit" | ||
| readme = "README.rst" | ||
| license.file = "LICENSE" | ||
| requires-python = ">=3.10" |
There was a problem hiding this comment.
Problem:
openlifu supports python 3.10+
However openlifu-sdk was set to support 3.12+
This means there will be no dependency version resolution for openlifu on python 3.10 and 3.11!
We need to either bump this up to 3.12 here, giving up on 3.10 and 3.11 forever,
or openlifu-sdk needs to back up and support 3.10, 3.11.
| @@ -74,6 +97,12 @@ | |||
| "sphinx_autodoc_typehints", | |||
| "furo>=2023.08.17", | |||
| ] | |||
There was a problem hiding this comment.
The test group now needs to be expanded to include the groups that are covered by test. Because the CI installs .[test] and so right now it wouldn't have what it needs to run the tests
2 participants
This is a fairly major change to the project, designed to address issues #8 and #10
openlifu-sdk)virtual_fitmodule intosegopenlifu.__init__, to prevent the many-seconds-long importing when accessing any submodule.pyproject.tomlthat allow for reduced dependency installation. Theopenlifu-test-appproject, for example, can package only the hardware-communication required dependencies by using the[io]option. The base application as used in Slicer should now be installed with the[app]option.Code that touches
openlifu.virtual_fitneeds to be refactored to useopenlifu.seg.virtualfit, and code that used things likeopenlifu.Transducerwill need to be changed to useopenlifu.xdc.Transducer