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

WIP: src layout for python #501

Closed
wants to merge 3 commits into from
Closed

WIP: src layout for python #501

wants to merge 3 commits into from

Conversation

konstin
Copy link
Member

@konstin konstin commented Apr 12, 2021

For a mixed project with the current layout, python will always pick the local folder over the installed version, causing a number of problems (#335 (comment)). In pure python projects, this can be solved with the src layout. For rust projects, cargo already occupies src, so I've named the folder python_src (name open to bikeshedding). With this change pyo3-src-layout/pyo3_src_layout/__init__.py and pyo3-src-layout/python_src/pyo3_src_layout/__init__.py both work.

I hope this doesn't collide with #489.

Closes #335

@messense
Copy link
Member

IMO python_src is a kinda ugly, why not just python?

@konstin
Copy link
Member Author

konstin commented Apr 20, 2021

My idea was this should work with something like the following layout:

my_tool
- src
  - lib.rs    Here's the core implementation
  - main.rs    The functionality exposed as cli
- python
  - src
    - lib.rs    The functionality exposed as pyo3 bindings
  - python_src
    - my_tool
      - __init__.py
      - addition_python_functionality.py
  - Cargo.toml     This crate contains the pyo3 bindings
- Cargo.toml    This crate contains the core functionality

This is similar to e.g. https://github.com/huggingface/tokenizers

I also thought that the _src suffix makes it clearer that the directory would be named src if it wasn't for cargo.

@konstin konstin force-pushed the python_src_folder branch from d03b559 to 9d4f106 Compare April 22, 2021 12:33
@konstin
Copy link
Member Author

konstin commented May 20, 2021

I had an idea for very different design which I think would be better.

The core idea is that for a pure rust project, it's a rust project (represented by Cargo.toml) but for a mixed project, it's a python project (represented by pyproject.toml) with an added rust submodule. For a pure rust project, still point -m to a Cargo.toml, for a mixed project point -m to a pyproject.toml. Mixed project would need to migrate to the following layout:

my-project
├── Readme.md
├── pyproject.toml
├── src
│   └── my_project
│       ├── __init__.py
│       └── bar.py
└── rust
    ├── Cargo.toml
    └── src
        └── lib.rs

This is more verbose than before, but it has the advantage of using the normal src layout for python while also having no confusion about the subfolder name (it's always rust). It also makes it clearer what is a submodule of what. The big disadvantage is that it would force everyone to restructure their projects.

@konstin konstin changed the title WIP: python_src layout WIP: src layout for python May 20, 2021
@messense
Copy link
Member

The core idea is that for a pure rust project, it's a rust project (represented by Cargo.toml) but for a mixed project, it's a python project (represented by pyproject.toml) with an added rust submodule. For a pure rust project, still point -m to a Cargo.toml, for a mixed project point -m to a pyproject.toml. Mixed project would need to migrate to the following layout:

my-project
├── Readme.md
├── pyproject.toml
├── src
│   └── my_project
│       ├── __init__.py
│       └── bar.py
└── rust
    ├── Cargo.toml
    └── src
        └── lib.rs

For existing Python projects adding a Rust extension to boost performance, this layout is quite nice. But for Rust projects adding a Python binding, it's a little annoying to have to move Rust source into a rust folder.

@davidhewitt
Copy link
Member

I think the overall proposal is quite agreeable. Do you think it'd be possible to make this the default "mixed layout" and add configuration options to allow users to customise the project layout a bit if they wanted? e.g. I think in Cargo.toml you can specify the lib.path to customise the Rust location, maturin can presumably be compatible with that.

The big disadvantage is that it would force everyone to restructure their projects.

For existing Python projects adding a Rust extension to boost performance, this layout is quite nice. But for Rust projects adding a Python binding, it's a little annoying to have to move Rust source into a rust folder.

I agree with both of these points. I wonder if some automated commands e.g. maturin migrate-project-layout and maturin make-mixed-project could be written to ease the pain?

  • migrate-project-layout it's possible to imagine a general command where the user can say where they want each of the Rust & python files to end up and maturin can do the shuffling around
  • make-mixed-project could move the existing repo contents into a Rust subfolder and add a pyproject.toml, __init__.py etc

@clbarnes
Copy link
Contributor

clbarnes commented Nov 8, 2021

As has been said maturin's value to the whole spectrum from "rust project with thin python bindings" to "python project with a few lines of rust" will always introduce a conflict.

Every change breaks a workflow, of course, but in my projects I tend to rely on rust tooling for tracking versions etc. (cargo-release for releases, then the rust core stores CARGO_ENV_VERSION and python fetches it to find the version), because it's so much better than python's tooling. Would tucking Cargo.toml into a subdirectory make that harder?

@messense
Copy link
Member

Closing in favor of #1185

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.

4 participants