Skip to content

Conversation

@hramezani
Copy link
Member

@hramezani hramezani commented Oct 31, 2025

Selected Reviewer: @samuelcolvin

@hramezani
Copy link
Member Author

@kschwab we are going to drop python3.9 support.

There is two tests in test_source_cli.py that need to be changed.

  • test_cli_metavar_format
  • test_cli_metavar_format_310

Could you please review and correct them? You can find the format errors in lint CI.
Please create a PR or simply paste your diff as a comment in my PR and I will apply them to my PR

@tschm
Copy link

tschm commented Oct 31, 2025

you can fix with 'uvx ruff check .'

@hramezani
Copy link
Member Author

you can fix with 'uvx ruff check .'

Some logic on those two tests that I mentioned above is related to Python < 3.9. It is not only about fixing some lint error.

We have a make format command in the repo that fixes all lint errors.

uv run ruff check --fix $(sources)

@kschwab
Copy link
Contributor

kschwab commented Nov 2, 2025

@hramezani, is the goal to remove Union from import? I can remove references, it just will extend the lists. e.g.:

@pytest.mark.parametrize(
    'value_gen,expected',
    [
        (lambda: str | int, '{str,int}'),
        (lambda: list[int], 'list[int]'),
        (lambda: List[int], 'List[int]'),  # noqa: UP006
        (lambda: list[dict[str, int]], 'list[dict[str,int]]'),
        (lambda: list[str | int], 'list[{str,int}]'),
        (lambda: list[str | int], 'list[{str,int}]'),
        (lambda: LoggedVar[int], 'LoggedVar[int]'),
        (lambda: LoggedVar[Dict[int, str]], 'LoggedVar[Dict[int,str]]'),  # noqa: UP006

        # Duplicate list above but with None
        (lambda: str | int | None, '{str,int}'),
        (lambda: list[int] | None, 'list[int]'),
        (lambda: List[int] | None, 'List[int]'),  # noqa: UP006
        (lambda: list[dict[str, int]] | None, 'list[dict[str,int]]'),
        (lambda: list[str | int] | None, 'list[{str,int}]'),
        (lambda: list[str | int] | None, 'list[{str,int}]'),
        (lambda: LoggedVar[int] | None, 'LoggedVar[int]'),
        (lambda: LoggedVar[Dict[int, str]] | None, 'LoggedVar[Dict[int,str]]'),  # noqa: UP006
    ],
)

Currently it is used for dynamic Union creation in the test cases. Thoughts?

@hramezani
Copy link
Member Author

@hramezani, is the goal to remove Union from import? I can remove references, it just will extend the lists. e.g.:

@pytest.mark.parametrize(
    'value_gen,expected',
    [
        (lambda: str | int, '{str,int}'),
        (lambda: list[int], 'list[int]'),
        (lambda: List[int], 'List[int]'),  # noqa: UP006
        (lambda: list[dict[str, int]], 'list[dict[str,int]]'),
        (lambda: list[str | int], 'list[{str,int}]'),
        (lambda: list[str | int], 'list[{str,int}]'),
        (lambda: LoggedVar[int], 'LoggedVar[int]'),
        (lambda: LoggedVar[Dict[int, str]], 'LoggedVar[Dict[int,str]]'),  # noqa: UP006

        # Duplicate list above but with None
        (lambda: str | int | None, '{str,int}'),
        (lambda: list[int] | None, 'list[int]'),
        (lambda: List[int] | None, 'List[int]'),  # noqa: UP006
        (lambda: list[dict[str, int]] | None, 'list[dict[str,int]]'),
        (lambda: list[str | int] | None, 'list[{str,int}]'),
        (lambda: list[str | int] | None, 'list[{str,int}]'),
        (lambda: LoggedVar[int] | None, 'LoggedVar[int]'),
        (lambda: LoggedVar[Dict[int, str]] | None, 'LoggedVar[Dict[int,str]]'),  # noqa: UP006
    ],
)

Currently it is used for dynamic Union creation in the test cases. Thoughts?

@kschwab Yes, the goal is to remove Union and Python3.9 related code and tests. we have some Union in the above two tests function body. that's why I mentioned them. I think we need to change those function test body.

@hramezani hramezani merged commit 4239ea4 into main Nov 7, 2025
16 checks passed
@hramezani hramezani deleted the drop-python39 branch November 7, 2025 10:59
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.

5 participants