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

Using pyproject-toml-rs #23

Closed
cnpryer opened this issue Apr 24, 2023 · 8 comments
Closed

Using pyproject-toml-rs #23

cnpryer opened this issue Apr 24, 2023 · 8 comments

Comments

@cnpryer
Copy link
Contributor

cnpryer commented Apr 24, 2023

Would you be open to using pyproject-toml?

@mitsuhiko
Copy link
Collaborator

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.

@cnpryer
Copy link
Contributor Author

cnpryer commented Apr 24, 2023

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
Copy link
Contributor Author

cnpryer commented Apr 24, 2023

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

@mitsuhiko
Copy link
Collaborator

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.

@cnpryer
Copy link
Contributor Author

cnpryer commented Apr 30, 2023

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"
]

@mitsuhiko
Copy link
Collaborator

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.

@cnpryer
Copy link
Contributor Author

cnpryer commented Apr 30, 2023

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.

@mitsuhiko
Copy link
Collaborator

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

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

No branches or pull requests

2 participants