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

refactor: apply future type hints style #1884

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

waketzheng
Copy link
Contributor

@waketzheng waketzheng commented Feb 14, 2025

Description

  1. Does not change any code logic
  2. Add extend-select for ruff lint
    "FA",     # https://docs.astral.sh/ruff/rules/#flake8-future-annotations-fa
    "UP",     # https://docs.astral.sh/ruff/rules/#pyupgrade-up
    "RUF100", # https://docs.astral.sh/ruff/rules/#ruff-specific-rules-ruf
  1. Run the following script to auto add from __future__ import annotations:
#!/usr/bin/env python
"""
Auto add 'from __future__ import annotations' to py files that imports `typing`
"""

from __future__ import annotations

import os
from pathlib import Path


def main() -> None:
    t = "from __future__ import annotations"
    package = Path(__file__).parent.name.replace("tortoise-orm", "tortoise")
    total = updated = 0
    for name in (package, "tests", "examples", "conftest.py"):
        if not (d := Path(name)).exists():
            print(f"Skip {d} as it not exists")
            continue
        for p in [d] if d.is_file() else d.rglob("*.py"):
            total += 1
            if t in (s := p.read_text()) or ("from typing " not in s and "import typing" not in s):
                continue
            updated += 1
            lines = s.splitlines()
            index = 0
            doc = False
            for idx, one in enumerate(lines):
                if one.startswith("#"):
                    continue
                if one.startswith('"""') or one.startswith("'''"):
                    if len(one) > 3:
                        # One line document
                        continue
                    if not doc:
                        doc = True
                        continue
                    doc = False
                    index = idx + 1
                    break
                elif not doc:
                    index = idx
                    break
            lines.insert(index, t)
            p.write_text(os.linesep.join(lines + [""]))
            print(f"Inject '{t}' into {p} at line {index + 1}")
    if not total:
        print("No target found, please check current directory!")
    elif not updated:
        print(f"None of file need to update({total} files found)")
    else:
        print(f"Summary: {total = }; {updated = }")


if __name__ == "__main__":
    main()
  1. Execute ruff check --fix --unsafe-fixes tortoise examples tests conftest.py to reformat
  2. Install autoflake and run its shell command to remove unused imports:
    autoflake --remove-all-unused-imports -i -r tortoise examples tests conftest.py
  3. Reformat by make _style
  4. Change make lint to be make style+make check
  5. Refactor tortoise/backends/base/schema_generator.py::BaseSchemaGenerator::_get_table_sql to reduce complexity (which was complained by codacy)

Motivation and Context

To improve contributing experience

How Has This Been Tested?

make ci

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

codspeed-hq bot commented Feb 14, 2025

CodSpeed Performance Report

Merging #1884 will not alter performance

Comparing waketzheng:type-hints (895c07a) with develop (5aae984)

Summary

✅ 16 untouched benchmarks

@coveralls
Copy link

coveralls commented Feb 14, 2025

Pull Request Test Coverage Report for Build 13408107803

Details

  • 440 of 463 (95.03%) changed or added relevant lines in 62 files are covered.
  • 5 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.01%) to 89.437%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tortoise/backends/oracle/client.py 5 6 83.33%
tortoise/contrib/aiohttp/init.py 0 1 0.0%
tortoise/contrib/fastapi/init.py 2 3 66.67%
tortoise/contrib/quart/init.py 0 1 0.0%
tortoise/contrib/sanic/init.py 0 1 0.0%
tortoise/contrib/starlette/init.py 0 1 0.0%
tortoise/converters.py 3 4 75.0%
tortoise/indexes.py 2 3 66.67%
tortoise/timezone.py 5 6 83.33%
tortoise/backends/base/schema_generator.py 95 97 97.94%
Files with Coverage Reduction New Missed Lines %
tortoise/backends/base_postgres/client.py 1 90.08%
tortoise/contrib/aiohttp/init.py 1 0.0%
tortoise/contrib/quart/init.py 1 0.0%
tortoise/contrib/sanic/init.py 1 0.0%
tortoise/contrib/starlette/init.py 1 0.0%
Totals Coverage Status
Change from base Build 13368119422: 0.01%
Covered Lines: 6588
Relevant Lines: 7186

💛 - Coveralls

@waketzheng
Copy link
Contributor Author

@henadzit Cloud you review this?

Copy link
Contributor

@henadzit henadzit left a comment

Choose a reason for hiding this comment

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

Hey, i reviewed probably a half of this PR and left a few comments. Hopefully, I'll check out the other half in the coming days.

@waketzheng waketzheng requested a review from henadzit February 18, 2025 09:09
Copy link
Contributor

@henadzit henadzit left a comment

Choose a reason for hiding this comment

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

Left a few minor comments but apart from that I think it's good to go!

@waketzheng waketzheng requested a review from henadzit February 19, 2025 09:42
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.

3 participants