Skip to content

[chore]: fix requirements to build whl#1436

Merged
deven367 merged 19 commits intomainfrom
fix-requirements-to-build-whl
Jan 15, 2026
Merged

[chore]: fix requirements to build whl#1436
deven367 merged 19 commits intomainfrom
fix-requirements-to-build-whl

Conversation

@deven367
Copy link
Collaborator

@deven367 deven367 commented Jan 14, 2026

This PR fixes the requirements so that the whl can be built during a release

  • migrates to group-dependency instead of project.optional-dependencies in pyproject.toml
  • update contributing to reflect the necessary changes
  • update all workflows to reflect the necessary changes

Copilot AI review requested due to automatic review settings January 14, 2026 15:26
Copy link

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 removes the docs optional dependency group from pyproject.toml and updates the build-docs workflow to install the mkdocstrings-parser package directly as a Git dependency. This change prevents pip from attempting to resolve the Git-based dependency during wheel builds, which was causing failures during release processes.

Changes:

  • Removed the docs optional dependency group containing the Git-based mkdocstrings-parser dependency from pyproject.toml
  • Updated the build-docs workflow to install mkdocstrings-parser directly via Git URL instead of through the [docs] extra

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
pyproject.toml Removed the docs optional dependency group to prevent Git dependencies from breaking wheel builds
.github/workflows/build-docs.yaml Updated to install mkdocstrings-parser directly as a Git dependency instead of via the removed [docs] extra

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

Copy link
Contributor

@elephaint elephaint left a comment

Choose a reason for hiding this comment

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

Why not via dependency-groups? Let's keep solutions same for all repos

elephaint

This comment was marked as duplicate.

Copy link
Contributor

@elephaint elephaint left a comment

Choose a reason for hiding this comment

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

PR Review Summary

Overall this is a clean modernization PR migrating to uv sync with dependency groups. A few items need attention:

Issue: Missing docs dependency group

CONTRIBUTING.md line 188 references --group docs, but this group doesn't exist in pyproject.toml. The old docs dependencies (mkdocstrings-parser) were merged into the dev group. Either:

  • Remove --group docs from CONTRIBUTING.md, OR
  • Create a separate docs group if keeping documentation deps isolated is desired

Minor: Inconsistent uv usage in lint.yaml

The lint workflow uses uv pip install --system pre-commit while other workflows use uv sync. Consider using uv sync --group dev for consistency, since pre-commit is in the dev group.

Note: CI coverage enforcement

CircleCI now runs with --no-cov, meaning coverage enforcement (80% threshold) relies solely on GitHub Actions' ubuntu/windows jobs. This is fine if intentional.

Consider: Dependency caching

None of the workflows cache uv/pip dependencies. Given the heavy ML dependencies, adding caching could significantly improve CI times.

Copy link
Contributor

@elephaint elephaint left a comment

Choose a reason for hiding this comment

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

Thanks @deven367

@deven367 deven367 merged commit b8d9864 into main Jan 15, 2026
21 checks passed
@deven367 deven367 deleted the fix-requirements-to-build-whl branch January 15, 2026 14:39
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