Repository navigation
Clear ty's 99 override errors - #976
Merged
Merged
Conversation
`make ty` (ty 0.0.89) failed with 99 diagnostics on develop. False positives (84): every `class Meta(<Base>.Meta)` in codex/models and codex/serializers trips invalid-attribute-override (astral-sh/ty#4711). A [[tool.ty.overrides]] block turns that rule off for those two trees only, as comicbox 5.3.2 does for its schemas. It also hides the deliberate Volume.name and CustomCover.library overrides there; the pyproject comment says to mark them with ty ignores when the block goes. Real (15): - permission_classes (7): AuthMixin declared Sequence[type[BasePermission]], narrower than DRF's Sequence[_PermissionClass], so every subclass that set its own classes clashed with APIView. Using DRF's type fixes all seven, lets three views drop their copied annotation, and retires three pyright reportIncompatibleVariableOverride ignores (AuthAPIView, AuthGenericAPIView, OpenSearch1View). - input_serializer_class (4): BrowserParamsView typed it type[BrowserSettingsSerializerBase], yet choices, metadata and mtime set a superclass or sibling serializer. Widened to their common base, BrowserFilterChoicesInputSerializer, which is all init_params needs; subclasses now inherit the type, dropping three pyright ignores. - NULL_CODE (1): the mixin is typed int | str, since VuetifyCharField compares against a str; drops a pyright ignore. - content_negotiation_class (1): djangorestframework-stubs types it str | None, but DRF resolves the setting to a class, so the override is right; ignored for both checkers with that reason. - Volume.name, CustomCover.library (2): deliberate type-changing model overrides, covered by the scoped override above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
make ty(ty 0.0.89) failed on develop with 99 diagnostics, all override checks. This PR gets it to pass without turning the rule off across the codebase.False positives (84)
Every
class Meta(<Base>.Meta)incodex/modelsandcodex/serializershitsinvalid-attribute-overridebecause of a ty bug (astral-sh/ty#4711). A[[tool.ty.overrides]]block inpyproject.tomlturns that rule off for those two trees only, the same approach comicbox 5.3.2 takes for its schemas. Views and everything else are still checked.Real fixes (15)
permission_classes(7):AuthMixindeclaredSequence[type[BasePermission]], narrower than DRF'sSequence[_PermissionClass], so every subclass that set its own permissions clashed withAPIView. Switching the mixin to DRF's type fixes all seven. Three views drop the narrow annotation they had copied.input_serializer_class(4):BrowserParamsViewtyped it astype[BrowserSettingsSerializerBase], but the choices, metadata and mtime views assign a superclass or sibling serializer. Widened it to their common base,BrowserFilterChoicesInputSerializer, which is allinit_paramsuses. Subclasses now inherit that type instead of re-declaring it.NULL_CODE(1): the mixin is typedint | str, becauseVuetifyCharFieldcompares against a string.content_negotiation_class(1): djangorestframework-stubs types itstr | None, but DRF resolves the setting to a class at runtime, so codex's override is correct. It gets an ignore for both checkers, with the reason in a comment.Volume.name,CustomCover.library(2): these change a field's type on purpose. The scoped override above also hides them. Addingty: ignorecomments now would itself failmake ty, since ty reports unused ignores as warnings, so the pyproject comment says to add them when the override is removed.The fixes also made six pyright
reportIncompatibleVariableOverrideignores unnecessary (AuthAPIView,AuthGenericAPIView,OpenSearch1View, the choices, metadata and mtime views, andVuetifyCharField), so they are removed.Reviewer notes
make lintstill shows two warnings: basedpyrightreportUnreachableatcodex/views/browser/annotate/cover.py:63and:69. That branch is deliberately kept under its "PENDING SCHEMA REMOVAL" note so dynamic and custom covers can come back with a revert, so this PR leaves it alone.Test plan
make fix: no changes beyond this diffmake lint: passes (only the two warnings above)make ty: all checks passedcc --min Candmi --min Bon the changed files: empty; complexipy passesmake test: 1668 pytest passed, 1 xfailed; 994 vitest tests across 97 files passed🤖 Generated with Claude Code