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

Convert Python examples to proper packages #5966

Merged
merged 60 commits into from
Apr 22, 2024
Merged

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Apr 12, 2024

What

This PR does the following:

  • Convert all Python examples to proper python packages, replacing the requirements.txt file with a pyproject.toml file.
  • Applies same conversion to tests/python/test_api.
  • Add WIP support for running example with pixi
  • Add a all_examples meta-project.
  • Fix dev-tools build-examples for the new example format.
  • Add a dev-tools build-examples install command to pip install the examples for the given channel (prerequisite to build-examples rrd).
  • Fix CI for the new example format.
  • Updates pixi to 0.19.1 in CI.

pixi support

This WIP pixi support allows running examples in the examples environment:

pixi run -e examples clock

Note that by default pixi builds the environment with a shim/empty/fake rerun package (from examples/python/_empty_rerun_sdk), which is a work-around to allow manual installation of rerun in the environment. As a result, at first (and whenever a rebuild is needed), this must be run:

pixi run -e examples py-build

all_examples

This is a meta-package that dynamically depends on all examples. The original hope was to be able to depend on it from pixi, avoiding the need to list example explicitly. However, this is current impossible for a number a reason:

all_examples also has a very basic CLI tool that allows listing all examples in a format suitable for pixi: all_examples list.

Also, it should be relatively easy to cleanly reimplement the functionality of run_all.py, which is currently broken:

As it stands, it allows for efficiently checking dependency conflicts, especially with uv:

cd /tmp
uv venv
source .venv/bin/activate
uv pip install -e path/to/rerun/examples/python/all_examples   # ok??

TODO

  • convert a few example to python package
  • initial version of a all_examples meta-package
  • fix re_build_example
  • RE-ENABLE LINK CHECKS!!!!
  • properly working pixi setup
  • python version matrix testing setup

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@abey79 abey79 added examples Issues relating to the Rerun examples 🐍 Python API Python logging API include in changelog labels Apr 12, 2024
Copy link

github-actions bot commented Apr 12, 2024

Deployed docs

Commit Link
b73dc19 https://landing-ea9s0unit-rerun.vercel.app/docs

@jleibs
Copy link
Member

jleibs commented Apr 12, 2024

First big snag:
the pixi.lock file ends up with:

      - pypi: file:///Users/hhip/src/rerun/examples/python/blueprint_stocks
      - pypi: file:///Users/hhip/src/rerun/examples/python/clock
      - pypi: file:///Users/hhip/src/rerun/examples/python/redirect

which obviously isn't portable.

@abey79
Copy link
Member Author

abey79 commented Apr 12, 2024

Right. I specifically used full paths because something required them (was it uv?). I'll try to push a little more with relative path then.

@jleibs
Copy link
Member

jleibs commented Apr 12, 2024

Removed those from the lock file and now I get an error:

thread 'main' panicked at src/install_pypi.rs:199:14:
could not convert path into uv dist: NotFound(Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/home/jleibs/rerun/file:/home/jleibs/rerun/examples/python/blueprint_stocks", query: None, fragment: None })

@abey79
Copy link
Member Author

abey79 commented Apr 13, 2024

Yes, there seem to be an issue with pixi: prefix-dev/pixi#1186

@abey79
Copy link
Member Author

abey79 commented Apr 13, 2024

Some though I had over the night: instead of the dependency list to dynamically change based on the current python version, it's probably better to just stick the example's required-python to the dependency:

"click @ file://path/to/clock ; python_version <= '3.11'"

This would make the dependency list less dynamic (basically just on example existence and skip = true), and reduce the caching-related annoyances.

@teh-cmc teh-cmc self-requested a review April 22, 2024 07:13
@teh-cmc
Copy link
Member

teh-cmc commented Apr 22, 2024

pixi support

This WIP pixi support allows running examples in the examples environment:

pixi run -e examples clock

Note that by default pixi builds the environment with a shim/empty/fake rerun package (from examples/python/_empty_rerun_sdk), which is a work-around to allow manual installation of rerun in the environment. As a result, at first (and whenever a rebuild is needed), this must be run:

pixi run -e examples py-build

all_examples

This is a meta-package that dynamically depends on all examples. The original hope was to be able to depend on it from pixi, avoiding the need to list example explicitly. However, this is current impossible for a number a reason:

* doing so means that examples would not be installed as editable, causing issue with `__file__` relative paths ([Examples: download datasets into OS standard cache directory instead of relative to the python file #6054](https://github.com/rerun-io/rerun/issues/6054))

* doing so we leak full path in the lock file

* pixi currently has an issue where indirect dependency with git dependencies are broken

all_examples also has a very basic CLI tool that allows listing all examples in a format suitable for pixi: all_examples list.

Also, it should be relatively easy to cleanly reimplement the functionality of run_all.py, which is currently broken:

* [Replace `run_all.py` and nox with better alternatives #6057](https://github.com/rerun-io/rerun/issues/6057)

As it stands, it allows for efficiently checking dependency conflicts, especially with uv:

cd /tmp
uv venv
source .venv/bin/activate
uv pip install -e path/to/rerun/examples/python/all_examples   # ok??

All of these explanations should appear in examples/python/README.md imo

@teh-cmc
Copy link
Member

teh-cmc commented Apr 22, 2024

detect_and_track_objects doesn't work for me, is that known?

Failed to run `python3 -m detect_and_track_objects --save /tmp/snippets/detect_and_track_objects.rrd`:
stdout:

stderr:
DEV ENVIRONMENT DETECTED! Re-importing rerun from: /home/cmc/dev/rerun-io/rerun/rerun_py/rerun_sdk
/home/cmc/dev/rerun-io/rerun/.pixi/envs/default/lib/python3.11/site-packages/transformers/utils/hub.py:124: FutureWarning: Using `TRANSFORMERS_CACHE` is deprecated and will be removed in v5 of Transformers. Use `HF_HOME` instead.
  warnings.warn(
[2024-04-22T07:33:00Z DEBUG re_log_types::data_table_batcher] creating new table batcher config=DataTableBatcherConfig { flush_tick: 1000000000s, flush_num_bytes: 131072, flush_num_rows: 18446744073709551615, max_commands_in_flight: None, max_tables_in_flight: None, hooks: BatcherHooks { on_insert: None, on_release: Some(ArrowChunkReleaseCallback("0x4f098020af0")) } }
[2024-04-22T07:33:00Z DEBUG re_sdk::recording_stream] setting recording info app_id=rerun_example_detect_and_track_objects rec_id=8fce6e69-bb27-432d-9020-e25dc4f7692b
[2024-04-22T07:33:00Z DEBUG re_log_encoding::file_sink] Saving file to "/tmp/snippets/detect_and_track_objects.rrd"…
[2024-04-22T07:33:00Z DEBUG re_sdk::recording_stream] setting recording info app_id=rerun_example_detect_and_track_objects rec_id=8fce6e69-bb27-432d-9020-e25dc4f7692b
/home/cmc/dev/rerun-io/rerun/.pixi/envs/default/lib/python3.11/site-packages/dataset/tracking_sequences/horses.mp4 already exists. No need to download
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/home/cmc/dev/rerun-io/rerun/.pixi/envs/default/lib/python3.11/site-packages/detect_and_track_objects.py", line 438, in <module>
    main()
  File "/home/cmc/dev/rerun-io/rerun/.pixi/envs/default/lib/python3.11/site-packages/detect_and_track_objects.py", line 432, in main
    track_objects(video_path, max_frame_count=args.max_frame)
  File "/home/cmc/dev/rerun-io/rerun/.pixi/envs/default/lib/python3.11/site-packages/detect_and_track_objects.py", line 330, in track_objects
    with open(COCO_CATEGORIES_PATH, encoding="utf8") as f:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/home/cmc/dev/rerun-io/rerun/.pixi/envs/default/lib/python3.11/site-packages/panoptic_coco_categories.json'
[2024-04-22T07:33:00Z DEBUG rerun_bindings::python_bridge] Shutting down the Rerun SDK
[2024-04-22T07:33:00Z DEBUG re_sdk::recording_stream] setting recording info app_id=rerun_example_detect_and_track_objects rec_id=8fce6e69-bb27-432d-9020-e25dc4f7692b
[2024-04-22T07:33:00Z DEBUG re_log_encoding::file_sink] Log stream written to /tmp/snippets/detect_and_track_objects.rrd

@abey79
Copy link
Member Author

abey79 commented Apr 22, 2024

detect_and_track_objects doesn't work for me, is that known?

Probably #6054. You need to install in editable mode.

Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

All the pixi stuff just worked for me, I could install and save rrd files for all examples without any issue; that's pretty great.

I don't quite follow how it all works but I'm more concerned about the fact that it does.

I'm not sure the complexity added by the all_examples thing really is worth it though.

examples/python/blueprint/pyproject.toml Outdated Show resolved Hide resolved
pixi.toml Show resolved Hide resolved
@abey79
Copy link
Member Author

abey79 commented Apr 22, 2024

I'm not sure the complexity added by the all_examples thing really is worth it though.

I agree, to me it's 50/50 if this thing should/will survive. I do want to keep it around for a bit and see how things pan out with a bit of hindsight.

Also, there is this bit of TODO left where all_examples may or may not play a role:

@teh-cmc
Copy link
Member

teh-cmc commented Apr 22, 2024

detect_and_track_objects doesn't work for me, is that known?

Probably #6054. You need to install in editable mode.

Doesn't pixi run dev-tools build-examples install --channel nightly do that for me already? All others worked it seems

@abey79
Copy link
Member Author

abey79 commented Apr 22, 2024

detect_and_track_objects doesn't work for me, is that known?

Probably #6054. You need to install in editable mode.

Doesn't pixi run dev-tools build-examples install --channel nightly do that for me already? All others worked it seems

From your error log, clearly the example was not installed in editable mode. I checked and it's build-examples install's fault. That's an oversight on my part—fixing. Good catch!

@abey79
Copy link
Member Author

abey79 commented Apr 22, 2024

All of these explanations should appear in examples/python/README.md imo

I added the pixi info, but skipped the all_examples one. Because #6054, running examples after pip install -e all_examples/. may be wonky/fail, so it's not best advertised too much. The speech about checking dependency conflict is present in all_examples/README.md though.

# Conflicts:
#	.github/workflows/contrib_checks.yml
#	.github/workflows/reusable_checks.yml
#	.github/workflows/reusable_checks_python.yml
#	.github/workflows/reusable_deploy_docs.yml
#	pixi.lock
#	pixi.toml
@abey79 abey79 merged commit 5cafbe3 into main Apr 22, 2024
38 of 39 checks passed
@abey79 abey79 deleted the antoine/python-examples-2.0 branch April 22, 2024 09:30
@emilk
Copy link
Member

emilk commented Apr 23, 2024

We have a lot of references to examples/python/…/main.py that this PR broke

emilk added a commit that referenced this pull request Apr 23, 2024
### What
While investigating recently broken links (because of
#5966 (comment)) I
decided to add a lint against linking to stuff on changing branches
(e.g. `/blobs/main`).

Unfortunately I cannot cover it all (see
#6077 for more), but this is an
OK start.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6078?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6078?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6078)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
abey79 added a commit that referenced this pull request Apr 23, 2024
@abey79 abey79 mentioned this pull request Apr 23, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Issues relating to the Rerun examples include in changelog 🐍 Python API Python logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants