-
-
Notifications
You must be signed in to change notification settings - Fork 649
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
announce (and warn about) end of Python 2.7 support starting in v2.24.x #21387
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this on
src/python/pants/backend/python/util_rules/interpreter_constraints.py
Outdated
Show resolved
Hide resolved
96e7d3b
to
f199786
Compare
441d51a
to
db26aac
Compare
Fixed what should be the final test failure at https://github.com/pantsbuild/pants/actions/runs/10785314963/job/29910366528?pr=21387#step:11:369 which is due to tests being skipped locally intentionally because not all Pythons were present on my system. Not sure why those tests passed ever because the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except for getting @memoized
working properly.
@memoized | ||
def inner(interpreter_constraints: tuple[str, ...]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the memoized
here won't do quite the right thing, as each call of warn_on_python2_usage_in_interpreter_constraints
will create a new inner
and thus a new batch of memoization.
I think the @memoized
needs to be a on a top level function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. The list
s being passed in to function in an earlier iteration are not hashable, so my brain was trying an "easy" fix to consolidate turning them into tuples for @memoized
to work. Will just reorganize them into two free functions so the @memorized
is technically on an outer function.
Announce that v2.23.x is the last Pants release series to proactively test that it works with Python 2.7.
Also, log a warning when Python 2 is detected in the default interpreter constraints or in an
interpreter_constraints
field. The warning can be disabled in case the user really knows what they want via the new--no-python-warn-on-python2-usage
option.