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

Add typing, remove deprecated RelatedLinkColumn #974

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

jieter
Copy link
Owner

@jieter jieter commented Nov 6, 2024

Writing union types as X | Y is first added in python 3.10, if we keep supporting it until the end of life of python 3.9, that would mean we can only merge this after 2025-10.

Alternatively, we can use the Union[X, Y] syntax which is already supported by python 3.9.

Another issue is Unpack which is available from python 3.11. It is part of typing_extensions, so we can still use it.

The final few errors in the current state of this PR is this list (in which the tests are not yet included in the type checking):

$ mypy django_tables2/
django_tables2/columns/base.py:327: error: Dict entry 0 has incompatible type "str": "tuple[str, Union[list[Any], tuple[Any, ...]]]"; expected "str": "Callable[..., str]"  [dict-item]
django_tables2/columns/base.py:329: error: Dict entry 0 has incompatible type "str": "Optional[Accessor]"; expected "str": "Callable[..., str]"  [dict-item]
django_tables2/columns/base.py:332: error: Argument 2 to "LinkTransform" has incompatible type "**dict[str, Callable[..., str]]"; expected "Optional[Accessor]"  [arg-type]
django_tables2/columns/base.py:332: error: Argument 2 to "LinkTransform" has incompatible type "**dict[str, Callable[..., str]]"; expected "Union[list[Any], tuple[Any, ...], None]"  [arg-type]
django_tables2/columns/base.py:388: error: Incompatible return value type (got "Union[SafeString, QuerySet[Any, Any]]", expected "SafeString")  [return-value]
django_tables2/columns/base.py:404: error: Argument 2 to "call_with_appropriate" has incompatible type "CellArguments"; expected "dict[str, Any]"  [arg-type]
django_tables2/columns/base.py:706: error: Incompatible types in assignment (expression has type "Union[Any, str, _StrPromise]", variable has type "str")  [assignment]
django_tables2/columns/manytomanycolumn.py:94: error: Argument 1 to "filter" of "ManyToManyColumn" has incompatible type "Union[SafeString, QuerySet[Any, Any]]"; expected "QuerySet[Any, Any]"  [arg-type]
django_tables2/rows.py:155: error: Incompatible types in assignment (expression has type "None", variable has type "str")  [assignment]
django_tables2/rows.py:169: error: Item "None" of "Optional[Any]" has no attribute "exists"  [union-attr]
django_tables2/rows.py:183: error: "BoundRow" has no attribute "column"  [attr-defined]
django_tables2/rows.py:184: error: Incompatible types (expression has type "BoundRow", TypedDict item "bound_column" has type "BoundColumn")  [typeddict-item]
django_tables2/views.py:172: error: Definition of "get_paginate_by" in base class "TableMixinBase" is incompatible with definition in base class "MultipleObjectMixin"  [misc]
django_tables2/views.py:245: error: "MultiTableMixin" has no attribute "request"  [attr-defined]
django_tables2/export/views.py:52: error: "ExportMixin" has no attribute "get_table"  [attr-defined]
django_tables2/export/views.py:52: error: "ExportMixin" has no attribute "get_table_kwargs"; maybe "get_dataset_kwargs"?  [attr-defined]
Found 16 errors in 5 files (checked 27 source **files)**

Ideally, users of django-tables2 should still be able to use def render(self, record) or def render(self, value) without complaints of the type checker. I think this might be hard to achieve, so we should document that using def render(self, **kwargs: Unpack[CellArguments]) and unpacking kwargs in the body of the method is the way to achieve type safety.

@jieter jieter force-pushed the feature/util-typing branch from 5e4cec8 to 88f7317 Compare December 21, 2024 10:14
@jieter jieter force-pushed the feature/util-typing branch from f501b82 to 0f7e5be Compare December 21, 2024 13:37
@jieter jieter force-pushed the feature/util-typing branch from b739c27 to 555071a Compare December 23, 2024 08:37
@mschoettle
Copy link
Contributor

With pyupgrade it should be easy to move from Union to | once Python 3.9 support gets dropped. It looks like the pre-commit config needs a small update to target 3.9 instead of 3.8.

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.

2 participants