Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constrain multiple BaseModelAdmin methods to return either list or tuple #1833

Merged
merged 2 commits into from
Nov 9, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions django-stubs/contrib/admin/options.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ _ListFilterT: TypeAlias = (
# Generic type specifically for models, for use in BaseModelAdmin and subclasses
# https://github.com/typeddjango/django-stubs/issues/482
_ModelT = TypeVar("_ModelT", bound=Model)
_DisplayT: TypeAlias = _ListOrTuple[str | Callable[[_ModelT], str | bool]]

class BaseModelAdmin(Generic[_ModelT]):
autocomplete_fields: _ListOrTuple[str]
Expand All @@ -87,7 +88,7 @@ class BaseModelAdmin(Generic[_ModelT]):
radio_fields: Mapping[str, _Direction]
prepopulated_fields: dict[str, Sequence[str]]
formfield_overrides: Mapping[type[Field], Mapping[str, Any]]
readonly_fields: _ListOrTuple[str] | None
readonly_fields: _ListOrTuple[str]
ordering: _ListOrTuple[str] | None
sortable_by: _ListOrTuple[str] | None
view_on_site: bool | Callable[[_ModelT], str]
Expand All @@ -107,18 +108,18 @@ class BaseModelAdmin(Generic[_ModelT]):
def formfield_for_manytomany(
self, db_field: ManyToManyField, request: HttpRequest, **kwargs: Any
) -> ModelMultipleChoiceField | None: ...
def get_autocomplete_fields(self, request: HttpRequest) -> Sequence[str]: ...
def get_autocomplete_fields(self, request: HttpRequest) -> _ListOrTuple[str]: ...
def get_view_on_site_url(self, obj: _ModelT | None = ...) -> str | None: ...
def get_empty_value_display(self) -> SafeString: ...
def get_exclude(self, request: HttpRequest, obj: _ModelT | None = ...) -> Sequence[str] | None: ...
def get_exclude(self, request: HttpRequest, obj: _ModelT | None = ...) -> _ListOrTuple[str] | None: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool! Previously this wasn't considered an error because str satisfies Sequence[str] 🙄

    def get_exclude(self, *args: Any, **kwargs: Any) -> str:
        return "a"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this is precisely what I ran in to: I wrote

ordering = "-date_created"

And was surprised when CI passed for mypy but not for system checks..

def get_fields(self, request: HttpRequest, obj: _ModelT | None = ...) -> _FieldGroups: ...
def get_fieldsets(self, request: HttpRequest, obj: _ModelT | None = ...) -> _FieldsetSpec: ...
def get_inlines(self, request: HttpRequest, obj: _ModelT | None) -> list[type[InlineModelAdmin]]: ...
def get_ordering(self, request: HttpRequest) -> Sequence[str]: ...
def get_readonly_fields(self, request: HttpRequest, obj: _ModelT | None = ...) -> Sequence[str]: ...
def get_ordering(self, request: HttpRequest) -> _ListOrTuple[str]: ...
def get_readonly_fields(self, request: HttpRequest, obj: _ModelT | None = ...) -> _ListOrTuple[str]: ...
def get_prepopulated_fields(self, request: HttpRequest, obj: _ModelT | None = ...) -> dict[str, Sequence[str]]: ...
def get_queryset(self, request: HttpRequest) -> QuerySet[_ModelT]: ...
def get_sortable_by(self, request: HttpRequest) -> Sequence[str]: ...
def get_sortable_by(self, request: HttpRequest) -> _DisplayT: ...
def lookup_allowed(self, lookup: str, value: str) -> bool: ...
def to_field_allowed(self, request: HttpRequest, to_field: str) -> bool: ...
def has_add_permission(self, request: HttpRequest) -> bool: ...
Expand All @@ -128,7 +129,6 @@ class BaseModelAdmin(Generic[_ModelT]):
def has_view_or_change_permission(self, request: HttpRequest, obj: _ModelT | None = ...) -> bool: ...
def has_module_permission(self, request: HttpRequest) -> bool: ...

_DisplayT: TypeAlias = _ListOrTuple[str | Callable[[_ModelT], str | bool]]
_ModelAdmin = TypeVar("_ModelAdmin", bound=ModelAdmin)
_ActionCallable: TypeAlias = Callable[[_ModelAdmin, HttpRequest, QuerySet[_ModelT]], HttpResponseBase | None]

Expand Down Expand Up @@ -202,9 +202,9 @@ class ModelAdmin(BaseModelAdmin[_ModelT]):
def get_action(self, action: Callable | str) -> tuple[Callable[..., str], str, str] | None: ...
def get_list_display(self, request: HttpRequest) -> _DisplayT: ...
def get_list_display_links(self, request: HttpRequest, list_display: _DisplayT) -> _DisplayT: ...
def get_list_filter(self, request: HttpRequest) -> Sequence[_ListFilterT]: ...
def get_list_select_related(self, request: HttpRequest) -> bool | Sequence[str]: ...
def get_search_fields(self, request: HttpRequest) -> Sequence[str]: ...
def get_list_filter(self, request: HttpRequest) -> _ListOrTuple[_ListFilterT]: ...
def get_list_select_related(self, request: HttpRequest) -> bool | _ListOrTuple[str]: ...
def get_search_fields(self, request: HttpRequest) -> _ListOrTuple[str]: ...
def get_search_results(
self, request: HttpRequest, queryset: QuerySet, search_term: str
) -> tuple[QuerySet[_ModelT], bool]: ...
Expand Down