Skip to content

Conversation

@jbcoe
Copy link

@jbcoe jbcoe commented Aug 26, 2025

This PR modernizes the use of pyproject.toml, migrating options from setup.py so that the project can be installed with uv sync and tests run with uv run pytest.

uv is a modern python package manger that encapsulates the use of virtual environments ensuring fast isolated builds.

See https://docs.astral.sh/uv/

This PR removes the ability to set the package name with an environment variable as modern pyproject.toml requires a static name: https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#name

@jbcoe jbcoe marked this pull request as ready for review August 26, 2025 23:11
@StephenOman
Copy link
Collaborator

The brew install cmake error on the macos-latest runners appears to be a GitHub issue: actions/runner-images#12912

@jbcoe
Copy link
Author

jbcoe commented Aug 31, 2025

The brew install cmake error on the macos-latest runners appears to be a GitHub issue: actions/runner-images#12912

I pushed a fix, if it works then we can pull it out into another standalone PR.

@jbcoe jbcoe force-pushed the jbcoe/migrate-to-pyproject-toml-and-uv branch 4 times, most recently from 45b882e to 4a2da8f Compare August 31, 2025 23:34
@jbcoe
Copy link
Author

jbcoe commented Aug 31, 2025

I've cleaned up the branch history so that there are just two meaningful PRs. I tend to squash-merge PR branches so would be happy to split this PR into two.

@StephenOman
Copy link
Collaborator

My personal preference is for single-issue PRs as a gift to my future self (although it's a preference I frequently ignore, and every time I do, I later regret).

Anyway, there is a fix in progress for the macos runner, so we may not need to do it ourselves, in which case we could just wait a few days.

@jbcoe jbcoe force-pushed the jbcoe/migrate-to-pyproject-toml-and-uv branch from 4a2da8f to a2d282e Compare September 1, 2025 18:21
@jbcoe
Copy link
Author

jbcoe commented Sep 1, 2025

I created #64 and simplified this PR.

@jbcoe
Copy link
Author

jbcoe commented Sep 2, 2025

There's some light duplication in pyproject.toml now


[project.optional-dependencies]
dev = [
  "pre-commit>=2.0.1",
  "isort>=5.13.2",
  "cmake_format>=0.6.10",
  "memory-profiler>=0.60.0",
  "pytest>=6.2.5",
  "pytest-benchmark>=3.4.1",
  "sphinx>=2.4.4",
  "sphinx-rtd-theme>=0.4.3",
  "setuptools>=69.5.1",
  "ruff>=0.4.3",
]
agent = ["torch>=1.3.1"]
all = [
  "pre-commit>=2.0.1",
  "isort>=5.13.2",
  "cmake_format>=0.6.10",
  "memory-profiler>=0.60.0",
  "pytest>=6.2.5",
  "pytest-benchmark>=3.4.1",
  "sphinx>=2.4.4",
  "sphinx-rtd-theme>=0.4.3",
  "setuptools>=69.5.1",
  "ruff>=0.4.3","torch>=1.3.1"
]

[dependency-groups]
dev = [
  "nle[dev]"
]

reading astral-sh/uv#9011, I think this is perhaps unavoidable unless we plan migrate away from the old-style optional dependencies. Keeping all in sync is now going to need manual work and checks. I'm not sure how to improve this or if it should block modernisation.

@jbcoe
Copy link
Author

jbcoe commented Sep 2, 2025

If we decide to use uv for CI and run tests with uv run pytest then we can drop dev from project.optional-dependencies and have it as a dependency-group only. Then there would be no need for all as we'd only have agent in project.optional-dependencies.

@jbcoe
Copy link
Author

jbcoe commented Sep 9, 2025

#69 should fix the cmake on MacOS issues. I'm happy to discuss how (and if) we want to proceed with this PR. Since folk recently re-ran the GitHub actions, I presume that there is some interest.

@jbcoe jbcoe force-pushed the jbcoe/migrate-to-pyproject-toml-and-uv branch from 5ae320b to 04c767e Compare September 9, 2025 12:58
@jbcoe jbcoe marked this pull request as draft September 9, 2025 13:21
@StephenOman
Copy link
Collaborator

Since folk recently re-ran the GitHub actions, I presume that there is some interest.

That was me. @heiner has also proposed we move to uv, so yes, there is interest.

@jbcoe jbcoe marked this pull request as ready for review September 17, 2025 11:05
@StephenOman
Copy link
Collaborator

If we decide to use uv for CI and run tests with uv run pytest then we can drop dev from project.optional-dependencies and have it as a dependency-group only. Then there would be no need for all as we'd only have agent in project.optional-dependencies.

I think this is what we should do. To keep both methods means maintaining that duplication, which looks wrong and un-pythonic to me.

@jbcoe
Copy link
Author

jbcoe commented Nov 8, 2025

Closing stale PR.

@jbcoe jbcoe closed this Nov 8, 2025
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.

2 participants