Basic support for Python Optional Dependencies #5940

samypr100 · 2023-02-21T14:11:28Z

Type

Bug fix (non-breaking change which fixes an issue): Fixes #
New feature (non-breaking change which adds functionality). Resolves #
Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves Make dash an optional dependency #5740

Motivation and Context

This PR is a more conservative approach to what's described in #5853. Using only [standard] came up as an option in the discussion so I've decided to adopt it in this alternate PR.

I've only included the gui dependencies as part of standard, but we can include the ML dependencies (from open3d-ml) as well as discussed in #5853 (comment) if we desire to.

Note, Including the ml dependencies would help close #2991 in addition to #5740.

Checklist:

I have run python util/check_style.py --apply to apply Open3D code style
to my code.
This PR changes Open3D behavior or adds new functionality.
- Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
  updated accordingly.
- I have added or updated C++ and / or Python unit tests OR included test
  results (e.g. screenshots or numbers) here.
I will follow up and update the code if CI fails.
For fork PRs, I have selected Allow edits from maintainers.

Description

As discussed in #5853, below is a table of the changes:

Optional Dependency	Contents
standard	Includes new (this PR) `requirements_gui.txt` file from https://github.com/isl-org/Open3D

Strictly speaking from a python dependencies size, below are the sizes with the new optional dependencies calculated on OSX (transitive sizes may vary between platforms).

Package	Transitive Dependencies Size (MB)
open3d	350.13
open3d[standard]	459.13

If we decide to include ml as part of standard, the above table becomes

Package	Transitive Dependencies Size (MB)
open3d	107.69
open3d[standard]	459.13

Total Savings

For table #1, the total savings would be about 109mb.

For table #2, the total savings would be about 351mb.

This change is

# Conflicts: # cpp/pybind/make_install_pip_package.cmake # python/requirements.txt

theNded · 2023-08-16T04:47:22Z

I am not an expert on CI. I had a conversation with the CI expert on our team, and we think your solution would be viable as long as we keep

workflows stable
artifacts correctly uploaded
docs consistent

I think we can start with this minimal PR first and check those items one by one until reach somewhere that looks promising. It's my pleasure to work with you!

fix: add missing new line to requirements_gui.txt

samypr100 · 2023-08-16T12:25:25Z

@theNded 👍 Can you re-run the CI on both PR's again with the latest commit? Thanks

docs consistent

For this, what do you have in mind? I believe I updated the docs unless I missed something? 😅

theNded · 2023-08-16T16:56:28Z

Nah, it is just a general comment for future reference. Docs look great now, although I haven't gone through them thoroughly.

samypr100 · 2023-08-17T03:02:10Z

@theNded Thanks! Looks like CI is passing on both PR's 🙂

theNded · 2023-08-17T03:27:28Z

@samypr100 thanks for the update!
Here are two of my thoughts before a thorough review:

Would it be possible that we keep the original installation behavior, i.e. pip install open3d installs the complete package, and pip install open3d[nogui] installs the partial package? That will probably make the transition smoother.
Would you be able to upgrade the workflows as well, such that both wheels are produced in the CI? For instance, right now, only one artifact is produced. We will need to upload both to github, upload both to gcloud, and distribute them with pypi correspondingly. This should involve modifications here.

Please correct me if I'm wrong in those comments; otherwise, I will be glad to help with the workflows.

samypr100 · 2023-08-17T13:56:11Z

@theNded In regards to the artifact question, only one artifact is needed to use optional dependencies. Extras is just metadata for the package that declares what to install in adition to the core required dependencies, giving more control to the end user. Hence luckily, there wouldn't be more than one artifact needed. Both open3d and open3d-cpu should get this functionality automatically without the need for a new artifact.

For your question about nogui, python spec doesn't support it since extras is only additive. There was a in-depth discussion on that on the other PR #5853 (review) with @johnthagen

johnthagen · 2023-08-17T14:08:38Z

@theNded Like @samypr100 said, the reasons for why extras can only be additive are explained in the link @samypr100 posted above.

But at the same time, many popular Python packages have gone through this transition due to how useful it is for end users to be able to better control/trim transitive dependencies. This can speed up development workflows, CI workflows, container image sizes, etc for all downstream users of Open3D who only need a subset of Open3D's functionality.

theNded · 2023-08-19T05:00:18Z

@samypr100 @johnthagen Thanks for the update, I have read the explanation and it looks very reasonable. I have a heavy workload right now at my full-time job, but hopefully I will be back to review this PR at the end of next week.

# Conflicts: # docs/getting_started.in.rst

theNded · 2023-08-27T23:21:01Z

python/open3d/__init__.py

@@ -109,7 +109,14 @@ def _insert_pybind_names(skip_names=()):
    sys.modules.update(submodules)


-import open3d.visualization
+try:


We should implement systematic unit tests starting from this point.
For example, in the pytest files, expect this exception conditioned on the extra package (open3d or open3d[standard]). This can help us make sure that the core behavior is preserved without the gui requirements. Might need to tweak some of the CIs, but a summary of some local tests would be also helpful.

This may not be quite critical for this PR, but if we plan on more advanced dependency management I think it would be required.

theNded · 2023-08-27T23:26:14Z

3rdparty/README_SYCL.md

@@ -98,7 +98,7 @@ Open3D is designed to make use of the SYCL GPU devices.

 ## List of oneAPI Python packages

-To make `pip install open3d` works out-of-the box on SYCL-enabled platforms,
+To make `pip install open3d[standard]` works out-of-the box on SYCL-enabled platforms,


Ideally, also add doc for the behavior of pip install open3d. Same below.

theNded · 2023-08-27T23:27:52Z

.github/workflows/windows.yml

@@ -399,7 +400,7 @@ jobs:
            $PIP_PKG_NAME=(Get-ChildItem open3d*-$py_tag-*.whl).Name
          }
          echo "Installing Open3D wheel $PIP_PKG_NAME in virtual environment..."
-          python -m pip install "$PIP_PKG_NAME"
+          python -m pip install "$PIP_PKG_NAME[standard]"


I feel like [all], [full], [complete], or even [gui] might be more informative. Standard gives me the impression that it is a standard minimal set. But this is possibly subjective. @ssheorey what's your opinion?

As an FYI, standard is used in another popular Python package:

https://www.uvicorn.org/#quickstart

Others use all:

https://github.com/tiangolo/fastapi/blob/a3f1689d78abef3d5d54ba655ec87cef81bf7384/pyproject.toml#L56

https://pandas.pydata.org/docs/getting_started/install.html#install-optional-dependencies

theNded · 2023-08-27T23:28:40Z

README.md

-pip install open3d       # or
-pip install open3d-cpu   # Smaller CPU only wheel on x86_64 Linux (v0.17+)
+pip install open3d[standard]       # or
+pip install open3d-cpu[standard]   # Smaller CPU only wheel on x86_64 Linux (v0.17+)


Same, please add the behavior of pip install open3d and pip install open3d-cpu. Ideally, elaborate on the design choice and the expected usage of both options.

theNded · 2023-08-27T23:29:56Z

cpp/pybind/make_install_pip_package.cmake

@@ -5,4 +5,4 @@
 #       it is guaranteed that there is only one wheel in ${PYTHON_PACKAGE_DST_DIR}/pip_package/*.whl
 file(GLOB WHEEL_FILE "${PYTHON_PACKAGE_DST_DIR}/pip_package/*.whl")
 execute_process(COMMAND ${Python3_EXECUTABLE} -m pip uninstall open3d open3d-cpu --yes)
-execute_process(COMMAND ${Python3_EXECUTABLE} -m pip install ${WHEEL_FILE} -U)
+execute_process(COMMAND ${Python3_EXECUTABLE} -m pip install "${WHEEL_FILE}[standard]" -U)


Can we expose this extra option to cmake/make as well?

theNded · 2023-08-27T23:35:19Z

docs/getting_started.in.rst

@@ -35,9 +35,12 @@ Pip (PyPI)

 .. code-block:: bash

-    pip install open3d        # or
-    pip install open3d-cpu    # Smaller CPU only wheel on x86_64 Linux (since v0.17+)
+    pip install open3d[standard]        # or


Same, could use some explanation.

theNded · 2023-08-27T23:40:06Z

util/ci_utils.sh

@@ -83,6 +83,7 @@ install_python_dependencies() {

    # TODO: modify other locations to use requirements.txt
    python -m pip install -r "${OPEN3D_SOURCE_ROOT}/python/requirements.txt"
+    python -m pip install -r "${OPEN3D_SOURCE_ROOT}/python/requirements_gui.txt"


Make this requirement optional (controlled by the script input flags).

theNded · 2023-08-27T23:48:15Z

python/setup.py

+# For extra requirements
+extra_requires = defaultdict(list)
+
+# Standard Extras


Could also put a link to the relevant PEP docs for reference.

theNded · 2023-08-27T23:48:43Z

python/open3d/__init__.py

+    import open3d.visualization
+except ModuleNotFoundError:
+    warnings.warn(
+        "Open3D Python GUI Libraries not found. "


NIT: GUI libraries

theNded · 2023-08-27T23:53:04Z

.github/workflows/windows.yml

@@ -276,6 +276,7 @@ jobs:
        run: |
          $ErrorActionPreference = 'Stop'
          python -m pip install -r python/requirements.txt
+          python -m pip install -r python/requirements_gui.txt


In practice the difference between open3d and open3d[standard] is gui packages, so naming feels fine.
But semantically this feels a bit inconsistent: open3d + gui = open3d[standard].

samypr100 force-pushed the optional-dependencies-gui-only branch from 9152501 to b1aab0a Compare March 8, 2023 20:30

johnthagen mentioned this pull request Mar 21, 2023

Cannot install Open3D 0.17.0 on MacOS 12 ARM #6015

Closed

3 tasks

johnthagen mentioned this pull request Apr 5, 2023

Support for Python Optional Dependencies #5853

Open

Basic support for Python Optional Dependencies

5ca0f74

samypr100 force-pushed the optional-dependencies-gui-only branch from b1aab0a to 5ca0f74 Compare April 5, 2023 14:40

theNded self-requested a review August 11, 2023 04:44

samypr100 added 4 commits August 11, 2023 15:46

Merge branch 'master' into optional-dependencies-gui-only

abd38d1

# Conflicts: # cpp/pybind/make_install_pip_package.cmake # python/requirements.txt

chore: small post merge doc update

0dc12e8

chore: apply check style again

14e35ea

Merge branch 'master' into optional-dependencies-gui-only

ba81b45

fix: add missing requirements_gui.txt to Dockerfile.wheel

78df9f9

fix: add missing new line to requirements_gui.txt

Merge branch 'master' into optional-dependencies-gui-only

01d1d86

# Conflicts: # docs/getting_started.in.rst

theNded requested changes Aug 27, 2023

View reviewed changes

johnthagen mentioned this pull request Jan 6, 2024

Open3d 0.16.1 fails to install correct dependencies under Poetry #5747

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic support for Python Optional Dependencies #5940

Basic support for Python Optional Dependencies #5940

samypr100 commented Feb 21, 2023 •

edited by germanros1987

Loading

theNded commented Aug 16, 2023 •

edited

Loading

samypr100 commented Aug 16, 2023 •

edited

Loading

theNded commented Aug 16, 2023

samypr100 commented Aug 17, 2023

theNded commented Aug 17, 2023 •

edited

Loading

samypr100 commented Aug 17, 2023

johnthagen commented Aug 17, 2023

theNded commented Aug 19, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

johnthagen Aug 28, 2023 •

edited

Loading

theNded Aug 27, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

theNded Aug 27, 2023

Basic support for Python Optional Dependencies #5940

Are you sure you want to change the base?

Basic support for Python Optional Dependencies #5940

Conversation

samypr100 commented Feb 21, 2023 • edited by germanros1987 Loading

Type

Motivation and Context

Checklist:

Description

Total Savings

theNded commented Aug 16, 2023 • edited Loading

samypr100 commented Aug 16, 2023 • edited Loading

theNded commented Aug 16, 2023

samypr100 commented Aug 17, 2023

theNded commented Aug 17, 2023 • edited Loading

samypr100 commented Aug 17, 2023

johnthagen commented Aug 17, 2023

theNded commented Aug 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnthagen Aug 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samypr100 commented Feb 21, 2023 •

edited by germanros1987

Loading

theNded commented Aug 16, 2023 •

edited

Loading

samypr100 commented Aug 16, 2023 •

edited

Loading

theNded commented Aug 17, 2023 •

edited

Loading

johnthagen Aug 28, 2023 •

edited

Loading