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

SCons: Integrate annotations as required import #99180

Closed
wants to merge 1 commit into from

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Nov 13, 2024

With 3.8 being the new minimum Python version, we can now utilize the extremely convenient from __future__ import annotations. Doing so adds functionality from PEP 5631, postponing the evaluation of type annotations away from define-time. In other words, a lot of hacky workarounds to setup type hints will no longer be necessary; the resulting syntax is much cleaner, even granting functionality which otherwise require 3.9 or 3.10 as their minimum Python version:

# Before
def some_function(env: "SConsEnvironment") -> Optional[Tuple[List[str], Union[int, float]]]:

# After
def some_function(env: SConsEnvironment) -> tuple[list[str], int | float]] | None:

Like the original PR, this adds from __future__ import annotations as a required import. While most files in our repo aren't using this functionality atm, it's still good practice for consistency & won't risk merge conflicts due to being at the very top of the import chain. To ensure that the aforementioned type-hint features are utilized, I've also added UP0062, UP0073, and UP0374 to the linter ruleset; because our repo never relied on the prior behavior, the first two "unsafe" rules have been marked as "safe" instead.

Besides the above, exactly one other change was made: adding version.py to the list of files excluded from ruff evaluation. I could've instead added an exclusion for the required-import to that file, but I'd rather keep it entirely isolated as it functions as a metadata container. Additionally, as all these modifications are entirely isolated from runtime, no functional changes will be introduced by this PR.

Footnotes

  1. https://peps.python.org/pep-0563/

  2. https://docs.astral.sh/ruff/rules/non-pep585-annotation/

  3. https://docs.astral.sh/ruff/rules/non-pep604-annotation/

  4. https://docs.astral.sh/ruff/rules/quoted-annotation/

@Repiteo Repiteo added this to the 4.4 milestone Nov 13, 2024
@Repiteo Repiteo requested a review from a team as a code owner November 13, 2024 16:22
@Repiteo Repiteo force-pushed the scons/future-annotations branch 2 times, most recently from f345cd3 to 99be7f3 Compare November 18, 2024 03:53
@Repiteo
Copy link
Contributor Author

Repiteo commented Nov 24, 2024

Superseded by #99640

@Repiteo Repiteo closed this Nov 24, 2024
@Repiteo Repiteo removed this from the 4.4 milestone Nov 24, 2024
@Repiteo Repiteo deleted the scons/future-annotations branch November 24, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant