Skip to content

Modernize Python tooling (uv, pyproject.toml, src layout, semantic release) - #246

Open
salman2013 wants to merge 6 commits into
openedx:masterfrom
salman2013:modernize-python-tooling
Open

Modernize Python tooling (uv, pyproject.toml, src layout, semantic release)#246
salman2013 wants to merge 6 commits into
openedx:masterfrom
salman2013:modernize-python-tooling

Conversation

@salman2013

@salman2013 salman2013 commented Aug 6, 2026

Copy link
Copy Markdown

Summary

  • Phase 1 — Migrate package metadata to pyproject.toml (PEP 621/639): setuptools-scm dynamic versioning, importlib.metadata for docs version, coverage + isort config, semantic-release changelog config. Delete setup.py, setup.cfg, .coveragerc.
  • Phase 2 — Switch from pip-tools to uv: dependency-groups (test-base/test/django42/quality/doc/ci/dev), uv.lock, rewrite tox.ini with tox-uv>=1/uv-venv-lock-runner, rewrite Makefile with direct uv run --group commands, update CI to astral-sh/setup-uv with workflow_call trigger. Delete requirements/.
  • Phase 3 — Add python-semantic-release workflow: release.yml runs CI, cuts releases via PSR v10.5.3, publishes to PyPI via OIDC trusted publisher. Delete pypi-publish.yml.
  • Phase 4 — Migrate to src/ layout: git mv code_annotations/ src/code_annotations/, update all filesystem paths in tox.ini, docs/conf.py, pyproject.toml, Makefile.

Ticket: openedx/public-engineering#519

Test plan

  • All CI matrix jobs pass (quality, docs, django42, django52)
  • Configure OIDC trusted publisher on PyPI for release.yml before merging

🤖 Generated with Claude Code

salman2013 and others added 4 commits August 6, 2026 15:45
- Add pyproject.toml with PEP 621 metadata, setuptools-scm dynamic
  versioning, coverage config, isort config, and semantic-release
  changelog config
- Add .. changelog-insertion-marker to CHANGELOG.rst for PSR
- Remove __version__ from __init__.py (unused at runtime)
- Update docs/conf.py to use importlib.metadata for version
- Delete setup.py, setup.cfg, and .coveragerc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add dependency-groups (test-base/test/django42/quality/doc/ci/dev)
- Add uv.lock (94 packages resolved)
- Add constraint-dependencies via edx_lint write_uv_constraints
- Rewrite tox.ini to use tox-uv>=1 and uv-venv-lock-runner
- Rewrite Makefile to use uv run tox and uv sync
- Rewrite ci.yml: setup-uv, uv sync --group ci, uv run tox,
  workflow_call trigger, matrix job name
- Delete requirements/

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add release.yml: runs CI via workflow_call, PSR v10.5.3, uploads
  dist artifact, publishes to PyPI via OIDC trusted publisher
- Delete pypi-publish.yml (replaced by release.yml)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move code_annotations/ to src/code_annotations/
- Add where = ["src"] to [tool.setuptools.packages.find]
- Update tox.ini quality commands to src/code_annotations paths
- Update docs/conf.py sphinx-apidoc path to src/code_annotations
- Update coverage omit path in pyproject.toml
- Rewrite Makefile to use direct uv run --group commands (no tox)
- Regenerate uv.lock

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Aug 6, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @salman2013!

This repository is currently maintained by @bmtcril.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Aug 6, 2026
@salman2013 salman2013 changed the title build: modernize Python tooling (uv, pyproject.toml, src layout, semantic release) Modernize Python tooling (uv, pyproject.toml, src layout, semantic release) Aug 6, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
requirements/doc.txt was removed in the uv migration, so RTD builds
were failing. Switch to installing via uv export and pip install.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@irfanuddinahmad irfanuddinahmad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified against actual current file content, live CI, and by actually running commands locally against this branch -- not just reading the diff. All CI checks are green, which is expected since the bugs below aren't caught by CI.

Same changelog: "false" bug as edx-lint#563 and api-doc-tools#409 -- identical release.yml pattern across all three, looks template-derived rather than repo-specific.

What's done correctly here and worth not re-litigating: license/SPDX handling (confirmed via an actual local build that it doesn't raise InvalidConfigError), the Django version-matrix uses the correct [tool.uv].conflicts pattern, isort is both declared and actually wired into the quality targets (unlike a sibling repo), no ruff scope-creep, and the OIDC publish + pre-flight tag/PyPI version check both check out.

with:
git_committer_name: "github-actions"
git_committer_email: "github-actions@github.com"
changelog: "false"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

changelog: "false" overrides the [tool.semantic_release.changelog] config this same PR carefully builds in pyproject.toml (mode = "update", matching insertion_flag) -- the action-level flag wins, silently disabling changelog generation entirely despite the config looking correct. This is the exact incident class that already caused real CHANGELOG.rst content loss in a sibling repo in this effort.

Comment thread pyproject.toml
{include-group = "ci"},
]

[tool.uv]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing default-groups = []. I reproduced this directly against the PR branch:

$ uv run --group django42 python -Wd -m pytest --collect-only -q
error: Groups `dev` (enabled by default) and `django42` are incompatible with the conflicts: {`code-annotations:dev`, `code-annotations:django42`}

This is make test-all's first line (Makefile), so that target is broken today for anyone running it locally. CI stays green only because tox-uv happens to pass --no-default-groups internally when driving tests through tox -- it masks the gap rather than avoiding it. uv.lock's own [manifest].conflicts list independently corroborates this: it enumerates django42 vs. test, doc, quality, and dev.

Comment thread .github/workflows/ci.yml
pull_request:
branches:
- "**"
workflow_call:

@irfanuddinahmad irfanuddinahmad Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This new workflow_call: trigger, combined with the pre-existing push: [master] trigger a few lines up (unchanged by this PR), means every push to master now runs this full matrix twice concurrently: once via the direct push trigger, once via release.yml calling this workflow as its run_tests job. Recommend dropping the push: [master] trigger now that release.yml owns that path.

@irfanuddinahmad

Copy link
Copy Markdown
Contributor

One more, on a file this PR doesn't touch so I can't anchor it inline: MANIFEST.in is untouched by this PR and still has:

include requirements/base.in
recursive-include code_annotations *.tpl *.html *.png *.gif *js *.css *jpg *jpeg *svg *py *.yaml *.yml

Line 1 references a file this PR deletes (requirements/base.in). Line 2 still hardcodes the pre-src/-layout path (code_annotations/ instead of src/code_annotations/). I built both a real sdist and wheel from this branch to check actual impact -- setuptools-scm's git-based file-finder pulls the right files into the sdist regardless of the stale line, and the wheel is governed by [tool.setuptools.package-data] (which was correctly updated), so this isn't a build breaker. Still worth fixing for correctness -- same "stale MANIFEST.in reference" pattern confirmed in a sibling repo (ccx-keys#190).

@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Aug 7, 2026
@mphilbrick211 mphilbrick211 moved this from Ready for Review to In Eng Review in Contributions Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Status: In Eng Review

Development

Successfully merging this pull request may close these issues.

4 participants