Skip to content

Conversation

waketzheng
Copy link
Contributor

@waketzheng waketzheng commented Feb 7, 2025

Summary

Only improve type hints, does not change any logic

  1. typing.xxx --> from typing import xxx
  2. Run ruff check --fix starlette tests
  3. from typing import ContextManager --> from contextlib import AbstractContextManager

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@Kludex
Copy link
Collaborator

Kludex commented Feb 7, 2025

Can we enforce those via ruff?

Copy link
Collaborator

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

If you can find ruff rules to enforce this behavior, I'm happy to accept it.

@waketzheng
Copy link
Contributor Author

waketzheng commented Feb 8, 2025

Run ruff check --select=UP --fix . can change from typing import Sequence to from collections.abc import Sequence, but it does not change the typing.Sequence. So I changed all typing.xxx to be from typing import xxx.

A mini demo to verify:

# bb.py
import typing
from typing import Callable, ContextManager, Sequence


def foo(a: Callable, b: ContextManager, c: Sequence) -> tuple[typing.Sequence, typing.ContextManager]:
    pass

Run ruff check bb.py will get the following output:

bb.py:2:1: UP035 [*] Import from `collections.abc` instead: `Sequence`
  |
1 | import typing
2 | from typing import Callable, ContextManager, Sequence
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP035
  |
  = help: Import from `collections.abc`

bb.py:2:1: UP035 `typing.ContextManager` is deprecated, use `contextlib.AbstractContextManager` instead
  |
1 | import typing
2 | from typing import Callable, ContextManager, Sequence
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP035
  |

Found 2 errors.
[*] 1 fixable with the `--fix` option.

Then run ruff check --fix bb.py, the file was updated to be:

import typing
from collections.abc import Sequence
from typing import Callable, ContextManager


def foo(a: Callable, b: ContextManager, c: Sequence) -> tuple[typing.Sequence, typing.ContextManager]:
    pass

@waketzheng waketzheng requested a review from Kludex February 11, 2025 03:14
@waketzheng
Copy link
Contributor Author

Can we enforce those via ruff?

Yes, ruff --select=UP . do that.

@Kludex
Copy link
Collaborator

Kludex commented Feb 11, 2025

Can we enforce those via ruff?

Yes, ruff --select=UP . do that.

But you replaced everything in this PR manually... Right? The diff duplicated since last time I checked.

@waketzheng waketzheng force-pushed the improve-type-hints branch from dcb657e to 7aefc73 Compare May 22, 2025 13:02
@waketzheng
Copy link
Contributor Author

@Kludex I use the following script to replace everything:

# refactor.py
"""
Change typing import style for python files
    'import typing' -> 'from typing import Any,cast,...'
    'typing.Any' -> 'Any'
    'typing.cast' -> 'cast'
    'typing.ContextManager' -> 'contextlib.AbstractContextManager'
"""

import re
from pathlib import Path

for d in ("starlette", "tests"):
    for p in Path(d).rglob("*.py"):
        s = p.read_text()
        t = "import typing"
        if t not in s:
            continue
        pattern = re.compile(r"(\W?)typing\.(\w+)(\W)")
        ws = set([i[1] for i in pattern.findall(s)])
        imports = "from typing import "
        for cm in ("ContextManager", "AsyncContextManager"):
            if cm not in ws:
                continue
            # typing.ContextManager -> contextlib.AbstractContextManager
            # typing.AsyncContextManager -> contextlib.AbstractAsyncContextManager
            ws.remove(cm)
            imports = f"from contextlib import Abstract{cm}\n" + imports
            s = re.sub(rf"(\W?)typing\.({cm})(\W)", r"\1Abstract\2\3", s)
        s = pattern.sub(r"\1\2\3", s)
        imports += ", ".join(sorted(ws))
        s = s.replace(t, imports)
        p.write_text(s)
        print(p, "updated.")

Command line is:

python refactor.py
./scripts/lint

@waketzheng waketzheng removed their assignment May 23, 2025
@Kludex
Copy link
Collaborator

Kludex commented May 27, 2025

I guess it's time to do this. It will make the code shorter, and more readable - also updating the packages to use collections module.

btw, same PR welcome on uvicorn if you are up to it. 😅

@Kludex Kludex merged commit 2ff7653 into encode:master May 27, 2025
7 checks passed
@waketzheng waketzheng deleted the improve-type-hints branch May 27, 2025 07:36
@waketzheng
Copy link
Contributor Author

btw, same PR welcome on uvicorn if you are up to it. 😅

encode/uvicorn#2638
Done.

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