Skip to content

Fix docs dependency metadata for PyPI - #271

Merged
CassNot merged 2 commits into
merlinquantum:mainfrom
ben9871:docs_fix
Jun 18, 2026
Merged

Fix docs dependency metadata for PyPI#271
CassNot merged 2 commits into
merlinquantum:mainfrom
ben9871:docs_fix

Conversation

@ben9871

@ben9871 ben9871 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

What changed:

Removed the Git URL dependency from pyproject.toml’s published docs extra.

Added that same Git dependency to docs/requirements.txt.

Updated docs CI to install from docs/requirements.txt with working-directory: docs.

Related Issue

Type of change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor / Cleanup
  • Performance improvement
  • CI / Build / Tooling
  • Breaking change (requires migration notes)

Proposed changes

pre_commit run --files pyproject.toml docs/requirements.txt .github/workflows/docs.yml: hooks skipped, no Python files.

Built wheel to temp dir: passed.

twine check on wheel: passed.

How to test / How to run

  1. Command lines
Block of code

Screenshots / Logs (optional)

Performance considerations (optional)

Documentation

  • User docs updated (Sphinx)
  • Examples / notebooks updated
  • Docstrings updated
  • Updated the API

Checklist

  • PR title includes Jira issue key (e.g., PML-126)

  • "Related Jira ticket" section includes the Jira issue key (no URL)

  • Code formatted (ruff format)

  • Lint passes (ruff)

  • Static typing passes (mypy) if applicable

  • Unit tests added/updated (pytest)

  • Tests pass locally (pytest)

  • Tests pass on GPU (pytest)

  • Test coverage not decreased significantly

  • Docs build locally if affected (sphinx)

  • With this command:

      > SPHINXOPTS="-W --keep-going -n" make -C docs clean html 
    

    the docs are built without any warning or errors.

  • New public classes/methods/packages are added in the API following the methodology presented in other files.

  • Dependencies updated (if needed) and pinned appropriately

  • PR description explains what changed and how to validate it

@CassNot

CassNot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Jun 18, 2026

Copy link
Copy Markdown

Claude finished @CassNot's task in 1m 13s —— View job


Review — Fix docs dependency metadata for PyPI

The core change is correct and worthwhile: PyPI rejects published metadata that contains direct URL / VCS dependencies, so moving sphinx-multiversion-contrib @ git+https://... out of the [docs] extra in pyproject.toml is the right call for a publishable wheel. The CI rewiring is also sound — with working-directory: docs, pip install -r requirements.txt resolves -e ..[docs] to the repo root and then installs the git dependency on the next line. ✅

However, the dependency was used by more than just pyproject.toml, and those other consumers were not updated — this introduces regressions.

🔴 1. Other install paths now break the docs build

docs/source/conf.py:95 unconditionally loads the extension:

extensions = [
    ...
    "sphinx_multiversion",   # requires sphinx-multiversion-contrib
    ...
]

Any html build (not just multiversion) imports it. After this PR, every entry point that relies on the [docs] extra alone no longer installs that package and will fail with Could not import extension sphinx_multiversion:

  • requirements-docs.txt:1-e .[docs] (repo-root file, not touched by this PR). This is the most direct regression — it duplicates docs/requirements.txt but is now incomplete. Fix this →
  • README.md:55pip install -e '.[docs]'
  • .github/PULL_REQUEST_TEMPLATE.md:87pip install -e .[docs] && make -C docs html — ironically, the very checklist command this PR's description references will now fail for contributors. Fix this →

Suggestion: pick a single source of truth. Either make requirements-docs.txt the canonical file (and have docs/requirements.txt reuse it, or delete one), and update README + PR template to pip install -r docs/requirements.txt. Right now there are two near-identical requirements files (requirements-docs.txt and docs/requirements.txt) that have drifted apart.

🟡 2. Pinning consistency

The git dependency is pinned to tag @1.1.1, which is good. Worth confirming this same package isn't also expected transitively elsewhere, and consider documenting in the PR why it must live outside the extra (PyPI direct-URL restriction) so a future contributor doesn't "helpfully" move it back into pyproject.toml.

Minor / nits

  • docs/requirements.txt mixes an editable local install (-e ..[docs]) with a normal requirement. This works, but the relative ..[docs] only resolves correctly when pip is run from docs/ — which the CI now guarantees via working-directory: docs. Anyone running it from the repo root will get a different result. A short comment in the file noting "run from docs/" would prevent confusion.

Summary

The PyPI fix is correct. Please also update the other three consumers of the docs extra (requirements-docs.txt, README.md, PR template) so contributors and any RTD-style builds don't break, and ideally de-duplicate the two requirements files.


Review only — no changes were made. Re-trigger with @claude and an explicit ask if you'd like me to apply the fixes above.
· branch docs_fix

@ben9871

ben9871 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Summary

What changed: • Removed the Git URL dependency from pyproject.toml’s published docs extra. • Added that same Git dependency to docs/requirements.txt. • Updated docs CI to install from docs/requirements.txt with working-directory: docs.

Related Issue

Type of change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor / Cleanup
  • Performance improvement
  • CI / Build / Tooling
  • Breaking change (requires migration notes)

Proposed changes

pre_commit run --files pyproject.toml docs/requirements.txt .github/workflows/docs.yml: hooks skipped, no Python files. • Built wheel to temp dir: passed. • twine check on wheel: passed.

How to test / How to run

  1. Command lines
Block of code

Screenshots / Logs (optional)

Performance considerations (optional)

Documentation

  • User docs updated (Sphinx)
  • Examples / notebooks updated
  • Docstrings updated
  • Updated the API

Checklist

  • PR title includes Jira issue key (e.g., PML-126)

  • "Related Jira ticket" section includes the Jira issue key (no URL)

  • Code formatted (ruff format)

  • Lint passes (ruff)

  • Static typing passes (mypy) if applicable

  • Unit tests added/updated (pytest)

  • Tests pass locally (pytest)

  • Tests pass on GPU (pytest)

  • Test coverage not decreased significantly

  • Docs build locally if affected (sphinx)

  • With this command:

      > SPHINXOPTS="-W --keep-going -n" make -C docs clean html 
    

    the docs are built without any warning or errors.

  • New public classes/methods/packages are added in the API following the methodology presented in other files.

  • Dependencies updated (if needed) and pinned appropriately

  • PR description explains what changed and how to validate it

@claude review the latest fix to the review feedback

@claude

claude Bot commented Jun 18, 2026

Copy link
Copy Markdown

Claude finished @ben9871's task in 1m 5s —— View job


Review — latest fix to the docs dependency feedback

  • Read the updated diff against origin/main
  • Verify the three previously-flagged consumers are fixed
  • Check the new single-source-of-truth approach resolves correctly
  • Post review

The latest changes address all three regressions I flagged in the previous review, and the chosen approach (a shared requirements-docs-vcs.txt) is cleaner than what I suggested. Nice. ✅

What's now correct

  • Single source of truth for the VCS dep. requirements-docs-vcs.txt holds the one pinned line:
    sphinx-multiversion-contrib @ git+https://github.com/Quandela/sphinx-multiversion-contrib.git@1.1.1
    
    and both requirements files include it via -r, so they can't drift again.
  • requirements-docs.txt-e .[docs] + -r requirements-docs-vcs.txt — the most direct regression is fixed. ✅
  • docs/requirements.txt-e ..[docs] + -r ../requirements-docs-vcs.txt. ✅
  • README.md:55 and .github/PULL_REQUEST_TEMPLATE.md:87 now use pip install -r requirements-docs.txt, both invoked from the repo root, so -e .[docs] and the -r line resolve correctly. ✅
  • pyproject.toml no longer carries the direct-URL dep, so the published wheel metadata is PyPI-clean (matches your twine check result). ✅
  • docs.yml install step gained working-directory: docs and uses pip install -r requirements.txt; the relative ..[docs] and ../requirements-docs-vcs.txt resolve against the repo root from there, consistent with the existing working-directory: docs on the build step. ✅

I traced each path-resolution case (CI from docs/, README/template from repo root) and they all land on the same repo-root editable install + the shared VCS file. conf.py:95 still loads sphinx_multiversion, and every entry point now installs it.

Remaining nits (non-blocking)

  • 🟡 Document why the dep lives outside the extra. Without a note, a future contributor may "helpfully" move sphinx-multiversion-contrib back into the [docs] extra and re-break PyPI publishing. A one-line comment at the top of requirements-docs-vcs.txt (e.g. # Kept out of pyproject [docs] extra: PyPI rejects direct-URL/VCS deps in published metadata) would prevent that. Fix this →
  • 🟡 docs/requirements.txt only resolves from docs/. -e ..[docs] / -r ../requirements-docs-vcs.txt assume the cwd is docs/. CI guarantees this via working-directory: docs, but a contributor running it from the repo root gets the wrong result. A short # run from docs/ comment would help. Fix this →
  • Two requirements files still co-exist. requirements-docs.txt and docs/requirements.txt now differ only by the relative-path prefix (. vs ..). The duplication that mattered (the VCS pin) is gone, so this is acceptable — just flagging that two install entry points remain. If you ever want one canonical file, the README/template could point at docs/requirements.txt too, but it's not required.

Verdict

The PyPI fix is correct and the prior regressions are resolved. The remaining items are documentation nits, not blockers — I'd consider this good to merge once you decide whether to add the explanatory comments.
· branch docs_fix

@CassNot
CassNot merged commit 041e1d7 into merlinquantum:main Jun 18, 2026
9 checks passed
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