-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Open3d 0.16.1 fails to install correct dependencies under Poetry #5747
Comments
It seems that all of the ML related dependencies are not being published through PyPI's JSON API so that Poetry can inspect them: Having a minimal non-ML package would also really help this overall situation: |
To work around this, I had to mirror all of the ML requirements that are missing from 0.16.1 JSON API: [tool.poetry.dependencies]
python = "^3.10, <3.11"
open3d = "0.16.1"
# See: https://github.com/isl-org/Open3D-ML/blob/8ddb67206e4fef55b39eea691ff00d49cef18be5/requirements.txt
addict = "*"
pillow = "*"
matplotlib = "*"
numpy = "*"
pandas = "*"
pyyaml = "*"
scikit-learn = "*"
tqdm = "*"
pyquaternion = "*" |
The Poetry devs pointed out that they are pulling the information from the PyPI JSON API. This currently is populated by the first wheel that is uploaded. If you look at: "requires_dist": [
"numpy (>=1.18.0)",
"dash (>=2.6.0)",
"nbformat (==5.5.0)",
"configargparse"
],
"requires_dist": [
"numpy (>=1.18.0)",
"dash (>=2.6.0)",
"nbformat (==5.5.0)",
"configargparse",
"addict",
"pillow (>=8.2.0)",
"matplotlib (>=3)",
"numpy (>1.15)",
"pandas (>=1.0)",
"pyyaml (>=5.4.1)",
"scikit-learn (>=0.21)",
"tqdm",
"pyquaternion"
], This is related to how this issue was solved: |
Okay, I've discovered the root cause of this issue. The ARM macOS wheels do not have the full list of requirements. If you unzip: And look at the
Since these were the first wheels uploaded, the JSON API stores those, and the x86 wheel dependencies are not locked properly by Poetry. |
Hi @johnthagen as far as I can tell, your debugging looks correct. The ARM macOS wheels do not support 3DML (yet) and so do not have the extra Open3D-ML requirements. From the next release, we will add 3DML support to macOS ARM and this issue should not occur. This is not related to the v0.16.1 bugfixes. |
@ssheorey Do you also think we could for the next release make the ML dependencies optional? Something like:
Similar for Dash:
To get everything:
This would make it much nicer to use Open3D in constrained, headless environments. |
@johnthagen thanks for the suggestion - we will consider it, but note that it does need significant effort in updating packaging and documentation. |
I was wondering if there was any way to use poetry + open3d at the moment? My company has been stuck on |
Hi @apockill based on @johnthagen 's debugging this seems to be a strange issue / race condition in the PyPI json API / poetry dependency checker, so I'm not really sure what we can do about this. If there is a fix for Open3D we would be happy to merge it. Otherwise, perhaps this should be addressed as a bug in poetry or PyPI json API. |
The way to make open3d work with poetry is for open3d to use the same set of requirements in all distributions, with variations expressed through pep508 markers eg
|
@ssheorey Here is a link the section of PEP 508 that describes environment markers: If we follow this line: Line 98 in 0f06a14
Here are the missing requirements: So I think the improvement needed would be to unconditionally include the ML dependencies, but put environment markers on them such that they are only installed for the wheels that include ML functionality on their platform (e.g. Related to this, it would be convenient if the user could opt-in using
I think that by using PEP 508 environment markers and |
Thank you for the quick response! I'm excited to get this working. I'm having a bit of trouble figuring out how to get this environment marker to work. I tried a few lines in my pyproject.toml, but had various syntax errors. I'm not sure if I'm missing something super obvious 😅 open3d = { version = "0.18.0", markers = "not-arm-64-dependency==1.0.0 and platform_machine != 'arm64'" } It seems like no matter where I put it, the No terminal matches 'n' in the current parser context, at line 1 col 1
not-arm-64-dependency==1.0.0 platform_ma
^
Expected one of:
* L_PAREN
* ESCAPED_STRING
* SINGLE_QUOTED_STRING
* MARKER_NAME I wasn't able to find documentation on this marker, so my trail ran cold. When I tried using just the |
@apockill This is something that the Open3D team would need to change about their built wheels, not something that an end user can fix externally. @dimbleby Was just showing an example with |
Hi @dimbleby thanks for the suggestion - I think we can make that work with some effort. The way dependencies are set are:
The change would be to add all requirement files, but update the environment markers based on the enabled components. We would appreciate it if someone wants to create a PR for this. I can provide help as needed. |
I inspected the 0.18 wheels, and it looks like the matrix of ML dependencies is a little more complex than just "not ARM":
@ssheorey Does this sound accurate? So the environment marker we'd need to apply to each ML dependency would look something like:
|
Related to this issue isl-org/Open3D#5747 Signed-off-by: aapozd <[email protected]>
Checklist
master
branch).Steps to reproduce the issue
Create a
pyproject.toml
file:Run:
Note that the way this mac-only package was published, it is missing ML-related dependencies in how Poetry pulls dependencies through the API:
For example,
scikit-learn
is not installed even though it is a dependency.Error message
Within the
poetry shell
:Open3D, Python and System information
Additional information
Discussion with Poetry devs about this topic:
The text was updated successfully, but these errors were encountered: