Skip to content

feat: modernize Python repo to uv + pyproject.toml - #377

Merged
feanil merged 1 commit into
openedx:masterfrom
farhan:farhan/modernize-python-repo
Nov 3, 2026
Merged

feanil merged 1 commit into
openedx:masterfrom
farhan:farhan/modernize-python-repo

Conversation

@farhan

@farhan farhan commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Important

Written with the assistance of Claude Code, human-reviewed before posting.

Modernize edx-submissions
Part of openedx/public-engineering#506

Summary

  • Repo publishes to PyPI: https://pypi.org/project/edx-submissions/
  • Move the package into a src/ layout (submissions/ → src/submissions/).
  • Replace setup.py/setup.cfg with pyproject.toml (PEP 621 static metadata).
  • Switch from pip-compile to uv with PEP 735 dependency groups; commit uv.lock.
  • Retain pylint/isort/pycodestyle as on master.
  • Move coverage config from .coveragerc into [tool.coverage.run] in pyproject.toml.
  • Update CI (ci.yml, check-reserved-keywords.yml, migrations-check.yml) to use astral-sh/setup-uv (SHA-pinned to v10.2.0) with uv sync --locked.
  • Add python-semantic-release + release.yml (OIDC trusted publishing), replacing pypi-publish.yml.
  • Drop Python 3.11 from tox; set requires-python = ">=3.12".

Removed/Updated

Deleted files: setup.py, setup.cfg, requirements/, .coveragerc, openedx.yaml (legacy file; ownership metadata lives in catalog-info.yaml), .github/workflows/pypi-publish.yml

Removed Makefile targets:

Target Reason
compile-requirements pip-compile is replaced by uv lock
piptools Only installed pip-tools for compile-requirements
$(COMMON_CONSTRAINTS_TXT) Downloaded requirements/common_constraints.txt; global constraints now come from edx_lint write_uv_constraints into [tool.uv].constraint-dependencies

Updated Makefile targets:

Target Change
dev_requirements uv sync --locked --group dev instead of pip install -r requirements/pip.txt + requirements/dev.txt
test_requirements uv sync --locked --group test instead of pip install -r requirements/test.txt
upgrade edx_lint write_uv_constraints pyproject.toml + uv lock --upgrade instead of make compile-requirements
test python -Wd -m pytest $(PYTEST_ARGS) (the command master's tox.ini ran) instead of bare pytest; tox.ini passes {posargs} through PYTEST_ARGS
test_quality pylint/isort paths point to src/submissions; setup.py dropped from isort args; --config=pycodestyle dropped (that file never existed; the config moved from setup.cfg to [pycodestyle] in tox.ini)

Updated workflows:

Workflow Change
ci.yml Name Python CI kept. Adds workflow_call (used by release.yml), removes the push: [master] trigger and fail-fast: false, runs uv sync --locked --group ci + uv run tox -e <env>
check-reserved-keywords.yml Runs uv run tox -e check_keywords instead of make check_keywords in a pip-installed env
migrations-check.yml uv sync --locked --group test, then uv pip install mysqlclient --no-binary mysqlclient (keeps master's source build of mysqlclient); drops the pip cache step
release.yml (new) Runs CI, then python-semantic-release, then gh release create, then OIDC publish to PyPI; no fetch-depth: 0, no secrets: inherit

Python 3.11 dropped

Master's tox.ini envlist was py{311,312}-django{42,52}-drf{315,latest}, but the CI matrix and classifiers were already 3.12-only. Master's setup.py had no python_requires; pyproject.toml now declares requires-python = ">=3.12".

Versioning

setuptools-scm with dynamic = ["version"] (fallback_version = "0.0.0.dev0"). Master had a PyPI publish workflow; python-semantic-release now sets the version from git tags at release time. src/submissions/__init__.py reads __version__ from importlib.metadata instead of hardcoding '4.0.2'. The v4.0.2 tag exists, so PSR will see the latest PyPI release.

Important Notes

  • tox: the envlist is django{42,52}-drf{315,latest}, quality, docs (the py factor is dropped because only 3.12 is tested). Each test env maps to its own dependency group in tox.ini; django52-drflatest uses the test group. tox -e <env> -- <pytest args> still works.
  • CI check names: the job name changed from Tests (ubuntu-latest, 3.12, <toxenv>) to <toxenv> (e.g. django42-drflatest). If branch protection requires the old check names, they need updating.
  • Codecov still uploads from django42-drflatest only, same as master.
  • fail-fast: false removed from the ci.yml matrix, so it now defaults to true: one failing matrix job cancels the rest.
  • Wheel contents: same files and same Requires-Dist as master's wheel, plus submissions/submissions.html. Master's wheel left that file out even though src/submissions/views/__init__.py renders it, so this is a fix.
  • Metadata: classifiers match master, except the License classifier, which is dropped in favour of the PEP 639 license = "AGPL-3.0-only". Authors are now Open edX Project, and the long description now comes from README.rst (master used a one-line string). No keywords are added.
  • Constraints: [tool.edx_lint].uv_constraints is empty: master's only local constraint (backports.zoneinfo;python_version<"3.9") does not apply on Python 3.12. The global constraints from master's common_constraints.txt are in [tool.uv].constraint-dependencies.
  • Dependency groups: the test group no longer pulls in the docs dependencies (master's requirements/test.in had -r docs.txt); Sphinx and friends live only in the doc group. The check_keywords tox env now uses the quality group (master used requirements/dev.txt).
  • Read the Docs: .readthedocs.yaml installs with uv sync --locked --group doc and builds with sphinx-build -W, so warnings still fail the build, as with master's fail_on_warning: true.
  • commitlint: commitlint.yml was already on master, so conventional commit format is already enforced.

Testing

Note: PR creator has tested the PR via CI and building the packages manually. No manual testing has been done to test the repo features. It will be great if the repo owner could test it.


🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.83%. Comparing base (ac35d1f) to head (580d325).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #377   +/-   ##
=======================================
  Coverage   95.82%   95.83%           
=======================================
  Files          23       23           
  Lines        3069     3070    +1     
  Branches      128      128           
=======================================
+ Hits         2941     2942    +1     
  Misses        115      115           
  Partials       13       13           
Flag Coverage Δ
unittests 95.83% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@farhan
farhan force-pushed the farhan/modernize-python-repo branch 5 times, most recently from c53cee9 to b741e29 Compare October 8, 2026 08:21
@farhan
farhan marked this pull request as ready for review October 8, 2026 15:01
@farhan
farhan force-pushed the farhan/modernize-python-repo branch 2 times, most recently from d317cea to d62bc7d Compare October 10, 2026 05:57

@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.

Before merging, a PyPI owner needs to add a trusted publisher for this repo with workflow release.yml. Earlier releases used an API token, so the first OIDC publish will fail without it.

Comment thread .github/workflows/ci.yml
name: ${{ matrix.toxenv }}
runs-on: ubuntu-latest
strategy:
fail-fast: 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.

Please restore fail-fast: false under strategy:. It was on master, and this migration should keep CI behaviour unchanged.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread .github/workflows/migrations-check.yml Outdated
pip uninstall -y mysqlclient
pip install --no-binary mysqlclient mysqlclient
uv sync --locked --group test
uv pip install mysqlclient --no-binary mysqlclient

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.

Please drop the uv pip install line and use uv run --with mysqlclient --no-binary-package mysqlclient python ./manage.py makemigrations --dry-run instead. uv pip install changes the venv outside uv.lock.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread .readthedocs.yaml Outdated
python:
install:
- requirements: requirements/docs.txt
commands:

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.

Please use RTD's native uv install instead of build.commands: restore the sphinx: block (configuration: docs/source/conf.py, fail_on_warning: true) and add python: install: - method: uv / command: sync / groups: [doc].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread .github/workflows/release.yml Outdated
dist/*

- name: Upload | Distribution Artifacts
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1

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.

Please bump the two artifact actions to sample-plugin's current pins: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 here and actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 in publish_to_pypi, so release.yml stays identical to sample-plugin (django-wiki already has them).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread .github/workflows/ci.yml
TOXENV: ${{ matrix.toxenv }}
run: tox
- name: Run tox
run: uv run tox -e ${{ matrix.toxenv }}

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.

Please use uv run --locked tox -e ${{ matrix.toxenv }}. Without --locked, uv run silently re-locks when uv.lock is stale instead of failing. It also matches i18n-tools, codejail-includes and platform-plugin-aspects.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think --locked with sync commands is enough.


- name: Check Keywords
run: make check_keywords
run: uv run tox -e check_keywords

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.

Please use uv run --locked tox -e check_keywords. Without --locked, uv run silently re-locks when uv.lock is stale instead of failing. It also matches i18n-tools, codejail-includes and platform-plugin-aspects.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think --locked with sync commands is enough.

@farhan
farhan force-pushed the farhan/modernize-python-repo branch 4 times, most recently from 7db5f10 to 0a4f68f Compare October 20, 2026 08:09
@farhan

farhan commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@irfanuddinahmad The PR is available for the next pass.

@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.

LGTM, thanks!

@farhan
farhan force-pushed the farhan/modernize-python-repo branch from 0a4f68f to db1cda9 Compare October 27, 2026 07:43
Comment thread pyproject.toml
]

[tool.setuptools]
include-package-data = 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.

nit: The commit message says the explicit *.html rule still ships submissions.html, but there's no package-data rule and the wheel doesn't have it, same as 4.0.2. Since the commit message becomes the squash message, can you drop that line?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 580d325. Dropped the *.html line from the commit message.

Part of openedx/public-engineering#506. Moves packaging to static
pyproject.toml metadata with a src/ layout, replaces pip-compile with
uv dependency groups and a committed uv.lock, and adds
python-semantic-release so releases publish via OIDC.

Test packages are kept out of the wheel: with setuptools-scm,
include-package-data = true would re-add every git-tracked file under
submissions/ as package data, shipping test files master's wheel never
had.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@farhan
farhan force-pushed the farhan/modernize-python-repo branch from db1cda9 to 580d325 Compare October 30, 2026 06:25
@farhan
farhan requested a review from feanil October 30, 2026 07:47
@feanil feanil changed the title chore: modernize Python repo to uv + pyproject.toml feat: modernize Python repo to uv + pyproject.toml Nov 3, 2026
@feanil
feanil merged commit 5ab5cb9 into openedx:master Nov 3, 2026
13 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.

3 participants