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

Support python 312 #6717

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

swheaton
Copy link

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes Support Python 3.12 #6433
  • 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 #

Motivation and Context

Closes #6433
Add support for python 3.12

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

@swheaton
Copy link
Author

Here is the start of my PR - although the checks do not pass, just opening it up for initial comment.

The current issue I'm seeing in CI is that I don't think there is a single version of scipy for requirements_test.txt that works with both python 3.12 and 3.8 -- current 1.10.1 requires Python <3.12, >=3.8, while the next release 1.11.1 requires Python <3.13, >=3.9.

Python 3.8 goes EOL in October of this year.

Here's a few options I see:

  • (don't recommend) Remove python 3.8 support despite it not being EOL for several more months
  • Wait to support python 3.12 (hold this PR) until 3.8 becomes EOL
  • (don't recommend) Don't pin packages for tests
  • Have 2 sets of test requirements: requirements_test_py312.txt that is used for python 3.12 instead of requirements_test.txt

I'm sure there may be other compatibility issues with packages but the CI just hasn't gotten far enough to see them yet.
@ssheorey

@ssheorey
Copy link
Member

Here is the start of my PR - although the checks do not pass, just opening it up for initial comment.

The current issue I'm seeing in CI is that I don't think there is a single version of scipy for requirements_test.txt that works with both python 3.12 and 3.8 -- current 1.10.1 requires Python <3.12, >=3.8, while the next release 1.11.1 requires Python <3.13, >=3.9.

Python 3.8 goes EOL in October of this year.

Here's a few options I see:

* (don't recommend) Remove python 3.8 support despite it not being EOL for several more months

* Wait to support python 3.12 (hold this PR) until 3.8 becomes EOL

* (don't recommend) Don't pin packages for tests

* Have 2 sets of test requirements: `requirements_test_py312.txt` that is used for python 3.12 instead of `requirements_test.txt`

I'm sure there may be other compatibility issues with packages but the CI just hasn't gotten far enough to see them yet. @ssheorey

Hi @swheaton thanks for contributing Python 3.12 support! See the requirements.txt file format docs for specifying different package versions based on different Python versions:

https://pip.pypa.io/en/stable/reference/requirements-file-format/

@swheaton
Copy link
Author

Hi @swheaton thanks for contributing Python 3.12 support! See the requirements.txt file format docs for specifying different package versions based on different Python versions:

https://pip.pypa.io/en/stable/reference/requirements-file-format/

Ah thanks, learned something today!

@swheaton
Copy link
Author

It goes further but looks like there are some C API changes in 3.12 that affect open3d. Googling around, this appears to be a common issue with python 3.12.
https://github.com/swheaton/Open3D/actions/runs/8423001029/job/23063629915#step:4:3856

This seems like a related S/O post (with no answer), since python/pyproject.toml also has jupyter related requirements in nit.

Seems related to cython/cython#5238, but I'm not sure what dependency to bump to get any available fixes there

@ssheorey ssheorey self-requested a review March 25, 2024 23:10
@ssheorey
Copy link
Member

TensorFlow updated to v2.16.1
PyTorch updated to v2.2.0
CUDA updated to 11.8

scipy==1.10.1; python_version < "3.12"
scipy==1.11.4; python_version >= "3.12"
tensorboard==2.13.0; python_version < "3.12"
tensorboard==2.16.2; python_version >= "3.12"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TensorBoard version must match the TensorFlow version. Supporting different tensorflow or tensorboard versions in a single release is not feasible.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. tensorflow version comes from that other file, ci_utils.sh. Might not be a clean way to match these if we differentiate between versions like this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requirements files are the "source of truth" for versions. For other places (e.g. CI), we just need to ensure that the versions match.

PS: Different versions of scipy are OK since we only consume the Python API, unlike with tensorboard / tensorflow where there is a much deeper C++ / Python interaction.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well tensorflow is not in any requirements file, that I can see at least. But I can make it match. I'm just saying, may not be that clean

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per my latest mainline comment, I am seeing now that it exists as a requirement in the Open3D-ML repo, so that will need to line up as well. But in the build scripts, the ML requirements files are only used some of the time

@swheaton
Copy link
Author

Note to self, also need to update open3d-ml repo, similar to isl-org/Open3D-ML#619
Since this repo always checks out the ML repo's main branch, I'm not sure the order these things are supposed to happen.

@johnthagen
Copy link
Contributor

Thanks for working on this! Python 3.12 support requests for Open3D will likely increase once Ubuntu 24.04 LTS is released on April 25 (in two weeks).

This is because Ubuntu 24.04 ships with Python 3.12 installed by default

@ssheorey ssheorey added this to the v0.19 milestone Apr 28, 2024
@ssheorey ssheorey added the build/install Build or installation issue label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/install Build or installation issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Python 3.12
3 participants