Skip to content

gh-115881: Document feature_version limitations #115980

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

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

hauntsaninja
Copy link
Contributor

@hauntsaninja hauntsaninja commented Feb 26, 2024

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

This looks great to me!

@@ -2180,12 +2180,13 @@ and classes for traversing abstract syntax trees:
modified to correspond to :pep:`484` "signature type comments",
e.g. ``(str, int) -> List[str]``.

Also, setting ``feature_version`` to a tuple ``(major, minor)``
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest adding maybe a modal box to highlight this a bit more?

``await`` as variable names. The lowest supported version is
``(3, 7)``; the highest is ``sys.version_info[0:2]``.
Setting ``feature_version`` to a tuple ``(major, minor)`` will result in
a "best-effort" attempt to parse using that Python version's grammar.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe also explicitly state that it's not guaranteed to disallow all constructs that weren't possible in a specific version?

Copy link
Member

Choose a reason for hiding this comment

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

I would even mention some of them:

  • with (current issue)
  • relaxed grammar for decorators
  • maybe something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a commit that expands what "best-effort" means, lysnikolaou let me know if that improves the explicitness!

I think I'd prefer not to get into details. I don't want users to expect that we list all divergences in behaviour here, nor do I think listing all divergences would be particularly useful for current users of feature_version.

Copy link
Member

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

LGTM!

@pablogsal pablogsal merged commit bea2795 into python:main Feb 29, 2024
@miss-islington-app
Copy link

Thanks @hauntsaninja for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington-app
Copy link

Sorry, @hauntsaninja and @pablogsal, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker bea2795be2c08dde3830f987830414f3f12bc1eb 3.12

@miss-islington-app
Copy link

Sorry, @hauntsaninja and @pablogsal, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker bea2795be2c08dde3830f987830414f3f12bc1eb 3.11

@bedevere-app
Copy link

bedevere-app bot commented Mar 1, 2024

GH-116173 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Mar 1, 2024
@bedevere-app
Copy link

bedevere-app bot commented Mar 1, 2024

GH-116174 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Mar 1, 2024
@hauntsaninja hauntsaninja deleted the gh-115881 branch March 1, 2024 01:50
@hauntsaninja hauntsaninja self-assigned this Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants