rye icon indicating copy to clipboard operation
rye copied to clipboard

Using pyproject-toml-rs

Open cnpryer opened this issue 2 years ago • 7 comments

Would you be open to using pyproject-toml?

cnpryer avatar Apr 24 '23 14:04 cnpryer

That library today does not do enough for what rye needs:

  • Rye needs to modify the toml file without losing formatting, that's why toml_edit is largely used.
  • Rye needs to access tool sections, which pyproject-toml does not give access to.

mitsuhiko avatar Apr 24 '23 14:04 mitsuhiko

I'm happy to work on upstream changes as well.

Rye needs to modify the toml file without losing formatting, that's why toml_edit is largely used.

I'm not sure if that's not something we couldn't upstream. I'd want this behavior as well from pyproject-toml. Maybe it could be feature-flagged similar to how toml handles this for its internals.

toml = { version = "0.7.3", features = ["preserve_order"] }

Rye needs access tool sections, which pyproject-toml does not give access to.

True and I'm curious if this is something that could be added upstream. If not this is how I implemented my core metadata struct

use pyproject_toml::{BuildSystem, Project, PyProjectToml as ProjectToml};
// ...

#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "kebab-case")]
/// The `Metadata` of a `Package`.
///
/// See https://peps.python.org/pep-0621/ for more about core metadata.
pub struct Metadata {
    /// The build system used for the `Package`.
    build_system: BuildSystem,
    /// The `Project` table.
    project: Project,
}

And to just add it to the PyProjectToml I just do this

/// A pyproject.toml as specified in PEP 621 with tool table.
#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "kebab-case")]
pub struct PyProjectToml {
    #[serde(flatten)]
    inner: ProjectToml,
    tool: Option<Table>,
}

Another thing I was personally going to think about was designing the metadata file management in a way similar to how cargo handles it (for Cargo.toml), but I'm not sure pyproject.toml data would play well with it, and I haven't thought enough about it to have any meaningful opinion.

For example, I really like how cargo makes path-based modifications to the toml using their LocalManifest mut utility.

https://github.com/rust-lang/cargo/blob/1c14e10523773908676ccdd7f0594ddd5058e387/src/cargo/util/toml_mut/manifest.rs#L263

cnpryer avatar Apr 24 '23 14:04 cnpryer

I've based my approach off of how maturin does it https://github.com/PyO3/maturin/blob/main/src/pyproject_toml.rs

cnpryer avatar Apr 24 '23 14:04 cnpryer

FWIW I'm super happy to use pyproject-toml. Happy to accept move this over when it covers that the moment the changes land. Having a good pyproject-toml library in Rust would really benefitial for everyone.

mitsuhiko avatar Apr 24 '23 15:04 mitsuhiko

When you mention

Rye needs to modify the toml file without losing formatting, that's why toml_edit is largely used.

Are you expecting to not make any formatting decisions on behalf of the user? For example, if I were to add enough dev dependencies, are you expecting the user to manage this

[project]
name = "test-project"
version = "0.0.1"
description = "Add a short description here"
authors = [
    { name = "Chris Pryer", email = "[email protected]" }
]
dependencies = ["xlcsv~=0.3.0", "packaging~=23.1", "pydantic~=2.0a3"]
readme = "README.md"
requires-python = ">= 3.8"
license = { text = "MIT" }

[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"

[tool.rye]
managed = true
dev-dependencies = ["pytest~=7.3.1", "black~=23.3.0", "ruff~=0.0.263", "mypy~=1.2.0", "polars~=0.17.9", "pandas~=2.0.1", "click~=8.1.3"]

What are your opinions on wrapping after a certain amount of elements in lists?

dev-dependencies = [
    "pytest~=7.3.1", 
    "black~=23.3.0", 
    "ruff~=0.0.263", 
    "mypy~=1.2.0", 
    "polars~=0.17.9", 
    "pandas~=2.0.1", 
    "click~=8.1.3"
]

cnpryer avatar Apr 30 '23 02:04 cnpryer

I haven't thought too much about it yet. In some ways the issue here is that this is a list to begin with rather than a table. This means that it's quite likely that poking around in that table is going to encourage reformatting that is bad for diffing. I wonder it there are any recommendations today. Personally I would probably try to format into a list of one dep per line.

mitsuhiko avatar Apr 30 '23 12:04 mitsuhiko

I wonder it there are any recommendations today

I can keep an eye out for that as well, but, personally, whenever I'm looking for ecosystem-aligning direction I like to defer to hatch's ways. Maybe there are better examples, but I've always felt like hatch has been pypa-aligned -- which is what I'd look to for recommendations like this.

cnpryer avatar Apr 30 '23 16:04 cnpryer

I'm closing this as it's unlikely that I will be working on this ticket.

mitsuhiko avatar Jan 21 '24 22:01 mitsuhiko