build: enable ruff rule sets F, PT, DJ, UP, and B by feanil · Pull Request #38291 · openedx/openedx-platform

feanil · 2026-04-06T22:18:16Z

Summary

This is PR 3 of the ruff migration for openedx-platform (replacing pylint + pycodestyle with ruff for static analysis).

PRs 1 and 2 enabled ruff E/W (pycodestyle) and I (isort) rules. This PR adds the remaining rule sets:

Rule set	Tool replaced / equivalent
`F`	pyflakes (unused imports, undefined names)
`PT`	flake8-pytest-style
`DJ`	flake8-django
`UP`	pyupgrade
`B`	flake8-bugbear

Strategy

All existing violations are suppressed with per-line # noqa comments (via ruff check --add-noqa), so the rules apply to new code going forward without requiring a mass fix of the existing codebase. The suppression commit is intentionally large and mechanical — reviewers can skip it and focus on the follow-up commits.

Commits

build: enable ruff rule sets F, PT, DJ, UP, and B — adds the rule sets to pyproject.toml
fix: suppress existing ruff F/PT/DJ/UP/B violations with noqa comments — mechanical --add-noqa run; ~3000 suppressions across the codebase
style: reformat long imports to satisfy ruff I001 — workaround for a ruff bug (astral-sh/ruff#24456): when --add-noqa adds a # noqa to an import line, ruff enforces parenthesised format for long imports in that block, triggering I001. Fixed by running ruff --fix for I001 (purely cosmetic wrapping, no reordering).
fix: remove unused override_waffle_flag import from test_proctoring.py — genuinely unused; removed rather than suppressed
fix: remove unused require_post_params import from user_authn login.py — genuinely unused; removed rather than suppressed
fix: remove unused imports from django_comment_client/tests/test_utils.py — genuinely unused; removed rather than suppressed
fix: convert format() call to f-string in conditional_block.py — --add-noqa injected # noqa: UP032 after a backslash line continuation, corrupting the syntax; fixed by converting to an f-string
style: suppress UP032 in helpers.py with block-level disable — same --add-noqa corruption in two .format() strings; fixed with # ruff: disable[UP032] / # ruff: enable[UP032] block suppression (smaller diff than f-string conversion)

Test plan

CI passes (ruff check, pylint, unit tests)
Verify ruff check --output-format=github . reports no errors locally

openedx/core/djangoapps/credit/views.py

@@ -101,7 +101,7 @@
            credit_request = create_credit_request(course_key, provider.provider_id, username)
            return Response(credit_request)
        except CreditApiBadRequest as ex:
-            raise InvalidCreditRequest(str(ex))  # lint-amnesty, pylint: disable=raise-missing-from
+            raise InvalidCreditRequest(str(ex))  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


openedx/core/djangoapps/content_libraries/rest_api/utils.py

        except api.BlockLimitReachedError as exc:
            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
+            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


openedx/core/djangoapps/content_libraries/rest_api/utils.py

        except api.InvalidNameError as exc:
            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
+            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


openedx/core/djangoapps/content_libraries/rest_api/utils.py

        except api.LibraryBlockAlreadyExists as exc:
            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
+            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


openedx/core/djangoapps/content_libraries/rest_api/utils.py

        except api.LibraryCollectionAlreadyExists as exc:
            log.exception(str(exc))
-            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from
+            raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


openedx/core/djangoapps/content_libraries/rest_api/libraries.py

@@ -362,7 +362,7 @@
        try:
            api.set_library_user_permissions(key, user, access_level=serializer.validated_data["access_level"])
        except api.LibraryPermissionIntegrityError as err:
-            raise ValidationError(detail=str(err))  # lint-amnesty, pylint: disable=raise-missing-from
+            raise ValidationError(detail=str(err))  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


openedx/core/djangoapps/content_libraries/rest_api/libraries.py

@@ -306,7 +306,7 @@
        try:
            api.update_library(key, **data)
        except api.IncompatibleTypesError as err:
-            raise ValidationError({'type': str(err)})  # lint-amnesty, pylint: disable=raise-missing-from
+            raise ValidationError({'type': str(err)})  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


openedx/core/djangoapps/content_libraries/rest_api/blocks.py

@@ -339,7 +339,7 @@
        try:
            version_num = api.set_library_block_olx(key, new_olx_str).version_num
        except ValueError as err:
-            raise ValidationError(detail=str(err))  # lint-amnesty, pylint: disable=raise-missing-from
+            raise ValidationError(detail=str(err))  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


lms/djangoapps/course_api/views.py

@@ -229,7 +229,7 @@
                msg = self.invalid_page_message.format(
                    page_number=page_number, message=str(exc)
                )
-                raise NotFound(msg)  # lint-amnesty, pylint: disable=raise-missing-from
+                raise NotFound(msg)  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


lms/djangoapps/course_api/views.py

@@ -195,7 +195,7 @@
                page_number=page_number, message=str(exc)
            )
            self.page.number = self.page.paginator.num_pages
-            raise NotFound(msg)  # lint-amnesty, pylint: disable=raise-missing-from
+            raise NotFound(msg)  # lint-amnesty, pylint: disable=raise-missing-from  # noqa: B904


Adds these rule sets to ruff's select list in pyproject.toml: - F (pyflakes: undefined names, unused imports, etc.) - PT (flake8-pytest-style: pytest best practices) - DJ (flake8-django: Django-specific checks) - UP (pyupgrade: modernise Python syntax) - B (flake8-bugbear: likely bugs and design issues) Existing violations are suppressed with per-line noqa comments in the follow-up commit; this commit just extends the configuration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Mechanical commit produced by: ruff check --select F,PT,DJ,UP,B --add-noqa . This suppresses all pre-existing violations so that the new rule sets land cleanly. New code added after this point will be checked by these rules without needing suppressions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

When a `# noqa` annotation is present on an import line inside a multi-group import block, ruff enforces a multi-line parenthesised format for long imports in that block. Wrapping those imports resolves the I001 reports introduced by the previous --add-noqa commit. Upstream issue: astral-sh/ruff#24456 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The import was genuinely unused — it only appeared on the import line with a pylint disable and noqa suppression but was never referenced in the test file. Removing it also cleans up the trailing blank line and lets ruff re-sort the import groups correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The import was genuinely unused — nothing in this file or downstream imports it from here. It was kept with a pylint disable and noqa suppression but serves no purpose. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…s.py Two imports were genuinely unused: - `Mock, patch` from unittest.mock: the file uses `mock.Mock()` and `mock.patch()` via the `mock` module imported on the line above, never the bare names. - `CommentClientMaintenanceError`: imported but never referenced in the file or re-exported to other modules. Both were kept with noqa suppressions that are no longer needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The UP032 noqa suppression added by the --add-noqa pass was placed on the continuation line of a backslash-split string literal, where ruff cannot see it (the violation is reported on the line where the string starts). Rather than restructuring the suppression, convert the two-line backslash-continued .format() call to an f-string, which fits within the 120-char line limit and removes the need for any suppression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

ruff's --add-noqa injected # noqa: UP032 after the backslash line continuation on two multi-line .format() strings in get_expiration_banner_text(), corrupting the syntax (a comment cannot follow a line continuation character). Rather than converting to f-strings, use ruff's block-level suppression (# ruff: disable[UP032] / # ruff: enable[UP032]) which avoids the syntax corruption and results in a smaller diff than an f-string rewrite. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The ruff --add-noqa pass added # noqa: <RULE> comments to all lines with existing violations. Pylint's C0301 (line-too-long) counts the full line including trailing comments, so these additions pushed 292 lines over 120 chars. Add per-line # pylint: disable=line-too-long to each of those lines. On lines that already had a # pylint: disable=<something> clause, extend the existing clause (e.g. # pylint: disable=raise-missing-from,line-too-long) rather than adding a second directive. Also removes # lint-amnesty, prefixes encountered during the edit pass -- these were temporary suppressions that are no longer needed. This suppression is temporary while both pylint and ruff are running. Once we drop pylint (PR 4+), all these per-line # pylint: disable=line-too-long comments can be removed as part of that cleanup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-advanced-security bot found potential problems Apr 6, 2026

View reviewed changes

feanil force-pushed the feanil/more_ruff_checks branch from 6356fc7 to c4298bd Compare April 8, 2026 01:21

feanil and others added 9 commits April 8, 2026 07:44

feanil force-pushed the feanil/more_ruff_checks branch from c4298bd to 6aeda5f Compare April 8, 2026 12:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: enable ruff rule sets F, PT, DJ, UP, and B#38291

build: enable ruff rule sets F, PT, DJ, UP, and B#38291
feanil wants to merge 9 commits intomasterfrom
feanil/more_ruff_checks

feanil commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

feanil commented Apr 6, 2026

Summary

Strategy

Commits

Test plan

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants