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

Ship rerun package inside of a rerun_sdk folder #1085

Merged
merged 4 commits into from
Feb 5, 2023
Merged

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Feb 3, 2023

The new layout inside of site-packages is now:

site-packages/
  rerun_bindings/           # The rust-bindings
    rerun_bindings.abi3.so
  rerun_bindings.libs/      # Portable libs for the rust bindings
    lib*.so
rerun_sdk/                  # Container folder
  rerun/                    # The rerun module
    __init__.py
    ...
rerun_sdk.pth               # Adds rerun_sdk to the path
rerun_sdk-0.1.0.dist-info/  # Package meta 

To make this all work, we had to move the rerun folder inside the rerun_sdk folder as it will appear when deployed. A little bit annoying for development, but still manageable and at least it reflects reality.

The biggest complication here was continuing to make maturin develop work as expected.
The problem is that maturin develop installs it's own .pth that wants to point to the top of the package.
This means it ends up pointing to rerun_py instead of rerun_sdk.

Sadly, .pth files don't chain, and maturin doesn't (yet) have a mechanism to allow us to override
the dev path that it adds for us. So, we work around this by adding a stub rerun package that reimports
the real one. It's a bit ugly, but only applies to folks running maturin dev and will never be seen by folks
working with installed package.

With these changes the following now works as exepcted:

$ pip install rerun
Collecting rerun
  Using cached rerun-1.0.30-py3-none-any.whl
Installing collected packages: rerun
Successfully installed rerun-1.0.30

$ pip install dist/rerun_sdk-0.1.0-cp38-abi3-manylinux_2_35_x86_64.whl 
Processing ./dist/rerun_sdk-0.1.0-cp38-abi3-manylinux_2_35_x86_64.whl
Requirement already satisfied: numpy>=1.23 in ./venv/lib/python3.10/site-packages (from rerun-sdk==0.1.0) (1.24.1)
Requirement already satisfied: pyarrow==10.0.1 in ./venv/lib/python3.10/site-packages (from rerun-sdk==0.1.0) (10.0.1)
Installing collected packages: rerun-sdk
Successfully installed rerun-sdk-0.1.0

$ python -m rerun
/home/jleibs/rerun/venv/bin/python: No module named rerun.__main__; 'rerun' is a package and cannot be directly executed

$ pip uninstall rerun
Found existing installation: rerun 1.0.30
Uninstalling rerun-1.0.30:
  Would remove:
    /home/jleibs/rerun/venv/bin/rerun
    /home/jleibs/rerun/venv/lib/python3.10/site-packages/rerun-1.0.30.dist-info/*
    /home/jleibs/rerun/venv/lib/python3.10/site-packages/rerun/*
Proceed (Y/n)? 
  Successfully uninstalled rerun-1.0.30

$ python -m rerun
2023-02-03T15:15:58.633010Z  INFO re_sdk_comms::server: Hosting a SDK server over TCP at 0.0.0.0:9876. Connect with the Rerun logging SDK.

TODO:

  • Tested on linux
  • Tested on mac
  • Tested on win

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I've added a line to CHANGELOG.md (if this is a big enough change to warrant it)

@jleibs jleibs marked this pull request as ready for review February 3, 2023 18:10
@jondo2010
Copy link
Contributor

Tested on Mac. All claims in the PR description work. One thing that we should possibly note in the installation docs: If I pip install rerun overtop of rerun-sdk:

➜ rerun --help                                                                                                                                                                                                                                     (base)
usage: rerun [-h] [--ignore IGNORE] [--interactive] [--verbose] [--version] command

Re-runs the given command every time files are modified in the current
directory or its subdirectories.
...

I get the other rerun instead of our rerun. This could be confusing to casual users.

@jleibs jleibs merged commit 220ba25 into main Feb 5, 2023
@jleibs jleibs deleted the jleibs/maturin_path2 branch February 5, 2023 04:20
@jleibs jleibs mentioned this pull request Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants