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

Adopt Python 3.7+ syntax #882

Merged
merged 2 commits into from Mar 16, 2022
Merged

Conversation

bhrutledge
Copy link
Contributor

Via pyupgrade --py37-plus. Closes #881.

I started another branch to adopt from __future__ import annotations, but found that support for abbreviated Union types won't be fully supported at runtime (e.g. in type aliases and cast) until Python 3.10. Rather than having multiple idioms for the same concept (e.g. str | None and Optional[str]), I found a workaround for the one instance where __future__ annotations was used.

@bhrutledge bhrutledge added this to the 4.0.0 milestone Mar 12, 2022
@@ -138,7 +136,7 @@ def password(self) -> Optional[str]:
# Workaround for https://github.com/python/mypy/issues/5858
return cast(Optional[str], self.auth.password)

def _allow_noninteractive(self) -> contextlib.AbstractContextManager[None]:
def _allow_noninteractive(self) -> "contextlib.AbstractContextManager[None]":
Copy link
Member

Choose a reason for hiding this comment

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

Why did this become quoted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I was referring to here:

I found a workaround for the one instance where __future__ annotations was used.

The quotes avoid a syntax error in Python < 3.9 (which introduces typed generics), similar to forward references:

twine/twine/settings.py

Lines 276 to 281 in a0ba32d

def from_argparse(cls, args: argparse.Namespace) -> "Settings":
"""Generate the Settings from parsed arguments."""
settings = vars(args)
settings["repository_name"] = settings.pop("repository")
settings["cacert"] = settings.pop("cert")
return cls(**settings)

My understanding is that __future__ annotations treats type annotations as strings, so this mirrors that behavior. It feels more consistent with the rest of the project, and avoids pyupgrade making other typing rewrites when that import is present.

@bhrutledge bhrutledge merged commit a6dd69c into pypa:main Mar 16, 2022
@bhrutledge bhrutledge deleted the 881-pyupgrade-37-no-future branch March 16, 2022 18: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.

Update syntax after dropping Python 3.6 support
2 participants