iree-org / iree

A retargetable MLIR-based machine learning compiler and runtime toolkit.
http://iree.dev/
Apache License 2.0
2.85k stars 618 forks source link

Python packages should declare their dependencies as requirements #5447

Open ScottTodd opened 3 years ago

ScottTodd commented 3 years ago

Discussed first on Discord here

I'm rewriting our docs and came across https://google.github.io/iree/get-started/getting-started-python#python-setup:

Install packages:

$ python -m pip install --upgrade pip
$ python -m pip install numpy absl-py

Each package should list its dependencies programmatically so we can drop that explicit pip install step in our documentation.

https://stackoverflow.com/questions/6947988/when-to-use-pip-requirements-file-versus-install-requires-in-setup-py and https://packaging.python.org/discussions/install-requires-vs-requirements/ suggest using the install_requires keyword in setup.py to list dependencies a project minimally requires to run correctly.

We use absl-py for flags and logging, mostly in tests but also in some util files. Similar story for numpy. Certain imports (I tested from iree.tf.support import module_utils but there may be others) fail without these dependencies installed.

ScottTodd commented 3 years ago

iree-tools-tf-snapshot also requires tensorflow (or tf-nightly). Probably a similar story for iree-tools-tflite-snapshot and iree-tools-xla-snapshot, but I haven't checked.

ScottTodd commented 3 years ago

I kept the "install packages" prerequisite in the new python bindings docs (tracking issue: https://github.com/google/iree/issues/5456). Would be nice to fix this to keep the docs simple.

stellaraccident commented 2 years ago

I think this has been fixed for a very long time. Closing.

ScottTodd commented 2 years ago

https://google.github.io/iree/bindings/python/#prerequisites still suggests installing

python -m pip install --upgrade pip
python -m pip install numpy absl-py
allieculp commented 2 years ago

@ScottTodd Should we give this a priority if still open? P1/2?

ScottTodd commented 2 years ago

This just needs someone to verify that requirements are set up correctly and then prune the docs. P2/P3.

aaron-schneider commented 1 year ago

Ho there! This bug hasn't been updated in a long time. Good intentions and all, but we're moving this to the backlog. Feel free to bring it back if you think there's a reasonable chance it'll get worked on in the next 6mo!