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

typing: fix dataclass_with_properties for static type checkers #821

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kukovecz
Copy link

Purpose of this PR:

Currently, all model in the codebase are implemented as dataclass with additional functionality provided by dataclass_with_properties. However, this approach causes static type checkers to fail in recognizing the dataclass structure.

To resolve this, I've introduced typing.dataclass_transform, which allows static type checkers to properly recognize and work with the enhanced dataclasses. This should improve type safety and make the code more robust from a type-checking perspective.

Demonstration:

Content of test.py:

import datetime

from spdx_tools.spdx.model import CreationInfo


def demo() -> CreationInfo:
    return CreationInfo(
        spdx_version="SPDX-2.3",
        spdx_id="SPDXRef-DOCUMENT",
        name="name",
        document_namespace="https://testdomain.com",
        creators=[],
        created=datetime.datetime.now()
    )

if __name__ == "__main__":
    demo()

Without this fix:

tools-python ❯ .venv/bin/pyright test.py
tools-python/test.py
  tools-python/test.py:6:15 - error: Expected class but received "(type[_T@dataclass]) -> type[_T@dataclass]" (reportGeneralTypeIssues)
  tools-python/test.py:8:22 - error: Expected 1 more positional argument (reportCallIssue)
2 errors, 0 warnings, 0 informations 

With this fix:

tools-python ❯ .venv/bin/pyright test.py
0 errors, 0 warnings, 0 informations 

Using dataclass_transform decorators, we can tell the static type
checkers that the class under construction is indeed a dataclass.

More info:
https://typing.readthedocs.io/en/latest/spec/dataclasses.html

Signed-off-by: János Kukovecz <[email protected]>
@kukovecz
Copy link
Author

kukovecz commented Sep 16, 2024

Ahhh, I’ve just realized that the typing.dataclass_transform was introduced in python 3.11, while this project supports and tests with python>=3.7.

I guess, close this PR then? Unless there's a plan to upgrade the minimum python version in the near future, or we can make it only run if python version is at least 3.11

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.

1 participant