Skip to content

Migrate from optional-dependencies to dependency-groups#168

Open
harupy wants to merge 2 commits intogithub:mainfrom
harupy:migrate-to-dependency-groups
Open

Migrate from optional-dependencies to dependency-groups#168
harupy wants to merge 2 commits intogithub:mainfrom
harupy:migrate-to-dependency-groups

Conversation

@harupy
Copy link
Contributor

@harupy harupy commented Jan 24, 2026

Summary

Migrates the Python SDK from pip's optional-dependencies pattern to uv's dependency-groups pattern for dev dependencies.

Changes

  • pyproject.toml: Changed [project.optional-dependencies] to [dependency-groups]
  • README.md: Updated installation instructions to use uv sync instead of pip install -e ".[dev]"
  • uv.lock: Regenerated with new dependency format

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
@harupy harupy requested a review from a team as a code owner January 24, 2026 08:57
Copilot AI review requested due to automatic review settings January 24, 2026 08:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the Python SDK’s dev dependency handling from pip/extras (optional-dependencies) to uv’s dependency groups, and updates documentation and lockfile accordingly.

Changes:

  • Switches from [project.optional-dependencies] to [dependency-groups] for Python dev dependencies in pyproject.toml.
  • Regenerates uv.lock to represent dev dependencies via dev-dependencies and metadata.requires-dev instead of extras.
  • Updates the Python README install instructions to reference uv sync rather than the old ".[dev]" extras pattern.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
python/pyproject.toml Defines a dependency-groups.dev group holding the former dev extras, aligning the project with uv’s dependency group model.
python/uv.lock Reflects the new dev dependency representation ([package.dev-dependencies] and [package.metadata.requires-dev]) for the github-copilot-sdk package.
python/README.md Updates installation instructions toward uv-based workflows, though the new pip example currently uses an invalid --group flag and needs correction; also note that CONTRIBUTING.md still references the old ".[dev]" extras pattern and should be updated separately.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

python/README.md Outdated

```bash
pip install -e ".[dev]"
pip install -e --group dev .
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated installation snippet mixes pip install with a --group flag, but pip itself doesn’t support --group—that flag is specific to uv and will cause this command to fail. To keep the instructions working after switching to dependency groups, this line should either use a valid pip command (without --group) or be rewritten to use uv in a way that correctly installs the dev group (similar to the uv sync example below).

Suggested change
pip install -e --group dev .
pip install -e .

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip >= v25.1 supports --group: https://pip.pypa.io/en/latest/news/#v25-1

@harupy
Copy link
Contributor Author

harupy commented Jan 31, 2026

@friggeri would you mind taking a look?

@friggeri
Copy link
Collaborator

friggeri commented Feb 3, 2026

@brettcannon do you mind opining?

pip install -e . --group dev
# or
uv pip install -e ".[dev]"
uv sync

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do note that due to the uv.lock file that is checked in that this is not equivalent to the command it's replacing.

@brettcannon
Copy link

brettcannon commented Feb 4, 2026

@brettcannon do you mind opining?

  1. The overall change is good (this sort of use is actually why we created dependency groups)
  2. https://github.com/github/copilot-sdk/blob/main/python/test-requirements.txt is now redundant
  3. I left a review comment about this, but uv sync is not the equivalent of the pip command as uv sync uses uv.lock while pip won't (it's a separate question as to why there is even a lock file to begin with)
  4. This is technically a breaking change as github-copilot-sdk[dev] no longer works, but I really doubt anyone will care (I only bring this up in case you're really strict on SemVer)

(Sorry for the brevity; sick toddler.)

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.

3 participants