chore: modernize Python tooling (uv, PEP 621, semantic-release, src layout) by salman2013 · Pull Request #265 · openedx/xblock-image-explorer

salman2013 · 2026-07-17T07:27:23Z

Summary

Phase 1: Migrate package metadata to pyproject.toml (PEP 621) — removed setup.py, added setuptools-scm for dynamic versioning, coverage config, importlib.metadata for __version__
Phase 2: Switch dependency management from pip-tools/requirements/*.txt to uv + uv.lock with [dependency-groups]; update tox.ini to use tox-uv/uv-venv-lock-runner; update CI to use astral-sh/setup-uv
Phase 3: Add python-semantic-release workflow for automated PyPI publishing via OIDC trusted publisher; add commitlint workflow to enforce conventional commits
Phase 4: Migrate to src/ layout — moved image_explorer/ to src/image_explorer/

Test plan

uv run tox -e quality — pylint 9.45/10, passes
uv run tox -e py312-django42 — 6/6 tests passed
uv run tox -e py312-django52 — 6/6 tests passed
Configure OIDC trusted publisher on PyPI for openedx/xblock-image-explorer (publisher: GitHub Actions, workflow: release.yml)

🤖 Generated with Claude Code

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

openedx-webhooks · 2026-07-17T07:27:29Z

Thanks for the pull request, @salman2013!

This repository is currently maintained by @Agrendalath.

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.

The translations/ directory is a symlink to conf/locale. Setuptools skips symlinks when building sdists, causing the wheel build to fail with "doesn't exist or not a regular file". Also remove stale requirements/ references from MANIFEST.in. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

farhan · 2026-07-21T09:27:40Z

 from .image_explorer import ImageExplorerBlock

-__version__ = '3.1.1'
+__version__ = version("xblock-image-explorer")


__version__ is assigned without a fallback. If the package is not installed, importlib.metadata.version() raises PackageNotFoundError and the import of this module fails entirely. Either:

Add a try/except PackageNotFoundError guard with a __version__ = "0.0.0" fallback, or

Remove __version__ entirely if no downstream code imports it (the simpler outcome).

fixed and remove version entirely if no downstream code imports it (the simpler outcome).

farhan · 2026-07-21T09:27:40Z

@@ -0,0 +1,140 @@
+[build-system]
+requires = ["setuptools>=64", "setuptools-scm>=8.0"]


We discussed about it before.

Should we add the minimum bound in setuptools>=64

farhan · 2026-07-21T09:27:40Z

+    "static/**",
+    "templates/**",
+    "public/**",
+    "conf/locale/**",


translations/config.yaml is present in the repo (src/image_explorer/translations/) and was included in the master wheel, but is missing from the PR wheel. Add "translations/**" to [tool.setuptools.package-data]:

[tool.setuptools.package-data] "image_explorer" = [ "static/**", "templates/**", "public/**", "conf/locale/**", "translations/**", ]

Because translations/ in this repo is a symlink (git mode 120000) pointing to conf/locale/. Setuptools cannot copy symlinks as package data — it fails with:

error: can't copy 'src/image_explorer/translations': doesn't exist or not a regular file

The actual files (like config.yaml) live in conf/locale/, which is already covered by the existing "conf/locale/" entry. So adding "translations/" would be redundant and break the wheel build.

Just reviewed the translations setup — translations/config.yaml is the atlas config used to pull translation files into the source repo (via make pull_translations in the Makefile). Atlas reads the source tree, not the installed wheel, so this file doesn't need to be in the package. Closing this comment.

https://github.com/salman2013/xblock-image-explorer/blob/modernize-python-tooling/src/image_explorer/conf/locale/config.yaml
https://github.com/salman2013/xblock-image-explorer/blob/modernize-python-tooling/Makefile

farhan · 2026-07-21T11:40:16Z

 name: Python CI

 on:
-  push:


Is this removal intentional?

@claude generated details:
The push: branches: [master] trigger was removed. Master CI used it to run checks on direct pushes to master; with only workflow_call added, direct pushes no longer trigger CI independently. Consider restoring the push: trigger alongside workflow_call:.

farhan · 2026-07-21T11:40:53Z

+
+      - name: Upload dist artifacts
+        if: steps.release.outputs.released == 'true'
+        uses: actions/upload-artifact@v4


actions/upload-artifact@v4 uses a mutable tag. All other actions in this PR are SHA-pinned — pin this one too.

Resolve the current SHA:

gh api repos/actions/upload-artifact/git/ref/tags/v4 --jq .object.sha

farhan · 2026-07-21T11:41:32Z

+
+    steps:
+      - name: Download dist artifacts
+        uses: actions/download-artifact@v4


same:

actions/download-artifact@v4 uses a mutable tag. Pin to a full SHA with a version comment, matching the pattern used for the other actions in this workflow.

farhan · 2026-07-21T11:42:54Z

-	$(PIP_COMPILE) -o requirements/ci.txt requirements/ci.in
-	sed -i '/^[dD]jango==/d' requirements/test.txt
+upgrade: ## update uv.lock and constraints with the latest packages
+	uv lock --upgrade


The upgrade target runs uv lock --upgrade before edx_lint write_uv_constraints. The standard order is edx_lint write_uv_constraints first (so constraints are updated), then uv lock --upgrade (so the lockfile respects them). Swap the two lines.

- Remove __version__ from __init__.py (nothing imports it) - Fix build-system requires to use setuptools-scm>8.1 - Add translations/** to package-data (was in old setup.py, missing from wheel) - Restore push: [master] trigger in ci.yml (needed for direct pushes) - Pin upload-artifact and download-artifact to full SHAs in release.yml - Fix upgrade target: write_uv_constraints before uv lock --upgrade Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…uild) translations/ is a symlink to conf/locale/ — setuptools can't copy symlinks as package data. The content is already covered by conf/locale/**. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

farhan · 2026-07-28T12:55:40Z

+[tool.semantic_release]
+# version is dynamic via setuptools-scm; no version_toml needed
+build_command = "python -m pip install --upgrade build && SETUPTOOLS_SCM_PRETEND_VERSION=$NEW_VERSION python -m build"
+allow_zero_version = true


I think repo already has version greater than zero, removing this config will be a great cleanup.

farhan · 2026-07-28T15:18:59Z

+        {group = "django42"},
+    ],
+]
+constraint-dependencies = []


The next line (constraint-dependencies = []) is empty — edx_lint write_uv_constraints was never run.

Without this, global edx-lint constraints (security pins, compatibility caps) are not applied to the lockfile. Run:

uv run --with edx-lint edx_lint write_uv_constraints pyproject.toml

Then commit the updated pyproject.toml and uv.lock.

farhan · 2026-07-28T15:18:59Z

 commands =
    mkdir -p var
    pytest {posargs:tests --cov image_explorer}



pytest is called directly here. Per the tooling flow, the Makefile should define commands and tox should call them:

Add to Makefile:
test: ## run tests mkdir -p var pytest $(posargs)

Change this commands line to make test.

farhan · 2026-07-28T15:19:00Z

 commands =
-    pylint --fail-under=9.0 image_explorer
+    pylint --fail-under=9.0 src/image_explorer



pylint is called directly here. Per the tooling flow, the Makefile should define commands and tox should call them:

Add to Makefile:
quality: ## run linting pylint --fail-under=9.0 src/image_explorer

Change this commands line to make quality.

farhan · 2026-07-28T15:19:00Z

+          path: dist/
+
+      - name: Publish to PyPI
+        uses: pypa/gh-action-pypi-publish@release/v1


The uses: on the next line (pypa/gh-action-pypi-publish@release/v1) is not SHA-pinned — all other actions in this PR use a full commit SHA. Pin it:

uses: pypa/gh-action-pypi-publish@<SHA> # release/v1

Get the current SHA:

gh api repos/pypa/gh-action-pypi-publish/git/ref/heads/release/v1 --jq '.object.sha'

- Remove allow_zero_version and major_on_zero from semantic_release config - Run edx_lint write_uv_constraints to populate constraint-dependencies - Add make test and make quality targets; tox now delegates to make - SHA-pin pypa/gh-action-pypi-publish in release workflow Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Move --cov=image_explorer into pytest addopts and simplify tox commands to plain `make test` to avoid make misinterpreting posargs flags (e.g. --cov) as its own options. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

salman2013 and others added 4 commits July 17, 2026 12:18

chore: migrate package metadata to pyproject.toml (PEP 621)

cad9ed8

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

chore: switch dependency management from pip-tools to uv

c0b4aa3

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

chore: add python-semantic-release and commitlint workflows

fa40927

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

refactor: migrate to src layout

49e3811

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

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 Jul 17, 2026

openedx-webhooks added this to Contributions Jul 17, 2026

github-project-automation Bot moved this to Needs Triage in Contributions Jul 17, 2026

salman2013 mentioned this pull request Jul 17, 2026

Modernize Python repo | XBlocks openedx/public-engineering#511

Open

16 tasks

mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Jul 20, 2026

mphilbrick211 requested a review from Agrendalath July 20, 2026 20:34

farhan self-requested a review July 21, 2026 09:08

farhan suggested changes Jul 21, 2026

View reviewed changes

salman2013 and others added 2 commits July 21, 2026 18:59

fix: remove translations/** from package-data (symlink breaks wheel b…

e4b6ca3

…uild) translations/ is a symlink to conf/locale/ — setuptools can't copy symlinks as package data. The content is already covered by conf/locale/**. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

salman2013 mentioned this pull request Jul 28, 2026

test: fix flaky video block JS tests openedx/xblocks-core#278

Closed

1 task

farhan suggested changes Jul 28, 2026

View reviewed changes

salman2013 and others added 2 commits July 30, 2026 15:06

fix: fix make test posargs breaking CI

f7ae64d

Move --cov=image_explorer into pytest addopts and simplify tox commands to plain `make test` to avoid make misinterpreting posargs flags (e.g. --cov) as its own options. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

		@@ -0,0 +1,140 @@
		[build-system]
		requires = ["setuptools>=64", "setuptools-scm>=8.0"]

Uh oh!

Conversation

salman2013 commented Jul 17, 2026

Summary

Test plan

Uh oh!

openedx-webhooks commented Jul 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants