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

Drop support for python 2.7, 3.4, and 3.5 #376

Merged
merged 16 commits into from
Feb 6, 2021
Merged

Drop support for python 2.7, 3.4, and 3.5 #376

merged 16 commits into from
Feb 6, 2021

Conversation

jdufresne
Copy link
Contributor

Fixes #305

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Me likey! We'd probably want to do a follow up, that cleans up the typing shenanigans we have here.

@jdufresne
Copy link
Contributor Author

We'd probably want to do a follow up, that cleans up the typing shenanigans we have here.

I would be happy to.

@@ -97,7 +106,7 @@ class _IndividualSpecifier(BaseSpecifier):

def __init__(self, spec="", prereleases=None):
# type: (str, Optional[bool]) -> None
match = self._regex.search(spec)
match = self._regex.search(spec) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, it isn't. I've removed the comment and am now defining _regex on the class with a type.

@jdufresne
Copy link
Contributor Author

We'd probably want to do a follow up, that cleans up the typing shenanigans we have here.

I started this work in #378. As a convenience, it builds off of this PR. If/when this lands, I'll go ahead and rebase it.

@pradyunsg
Copy link
Member

Wheeee! So many PRs and commits to review! Thanks a lot for pushing this forward @jdufresne, it is genuinely appreciated.

I'll catch up with the rest of the PRs tomorrow, but feel free to @-mention me in case I miss some PR. :)

Copy link
Member

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! A change that was propagated throughout test_tags does need to be reverted, though.

tests/test_tags.py Outdated Show resolved Hide resolved
@brettcannon brettcannon self-requested a review January 5, 2021 18:01
Base automatically changed from master to main January 21, 2021 19:20
@ShadowJonathan
Copy link

@di @brettcannon @pradyunsg what is this PR waiting for?

@di
Copy link
Member

di commented Jan 29, 2021

See my comment here: #385 (comment), although now that 20.9 is out this can probably be merged.

@brettcannon
Copy link
Member

@ShadowJonathan we are tracking the discussion at #305

@brettcannon
Copy link
Member

@jdufresne would you mind taking one more pass for your PR to pick any new code/changes that came in since you did this work? I'm also fine with not merging anymore PRs until this goes in if you are up for doing this.

/cc @pradyunsg about possibly pausing PR merging until this gets done to prevent anymore skew.

With Python 2 support dropped, these imports are no longer necessary.
In Python 3, all files are utf-8 by default.
With Python 2 support dropped, the fallbacks are not required.
As the package no longer supports Python 2, the wheel is not universal.
With Python 2 support dropped, these compatibility shims are no longer
necessary.
Also restore `if` statement guards.
@jdufresne
Copy link
Contributor Author

@brettcannon

I rebased and added two new commits:

  • Rerun pyupgrade
  • Simplify formatting repr strings

@brettcannon
Copy link
Member

@di @pradyunsg I refreshed our review statuses as I think enough has changed to warrant another look.

@jdufresne thanks for the updates! I'm hoping to review this week.

packaging/markers.py Outdated Show resolved Hide resolved
packaging/markers.py Outdated Show resolved Hide resolved
packaging/markers.py Outdated Show resolved Hide resolved
packaging/requirements.py Outdated Show resolved Hide resolved
packaging/requirements.py Outdated Show resolved Hide resolved
packaging/requirements.py Outdated Show resolved Hide resolved
packaging/specifiers.py Outdated Show resolved Hide resolved
packaging/version.py Outdated Show resolved Hide resolved
packaging/version.py Outdated Show resolved Hide resolved
@brettcannon
Copy link
Member

@jdufresne I've approved the PR, but there are a couple of small tweaks that could be made if you're up for it while we wait to see if anyone else wants to review this before it's merged.

packaging/requirements.py Outdated Show resolved Hide resolved
packaging/requirements.py Outdated Show resolved Hide resolved
Co-authored-by: Brett Cannon <brett@python.org>
packaging/version.py Outdated Show resolved Hide resolved
@jdufresne
Copy link
Contributor Author

Thanks for the review Brett and all. I've applied all suggestions.

CHANGELOG.rst Outdated Show resolved Hide resolved
Co-authored-by: Pradyun Gedam <3275593+pradyunsg@users.noreply.github.com>
@brettcannon
Copy link
Member

It's been about a week since the pip release with packaging 20.9 went out and there haven't been any macOS issues filed here or on pip that I can tell. That suggests to me that we are in the clear to take this and bid adieu to Python 2.7 (and 3.4 and 3.5)! 🎉

@brettcannon brettcannon merged commit 4d60352 into pypa:main Feb 6, 2021
@jdufresne jdufresne deleted the drop-py2 branch February 6, 2021 18:58
pradyunsg pushed a commit that referenced this pull request Dec 19, 2023
The ignore was added in #1, and was migrated from `.coveragerc` to the `pyproject.toml` in #586, even though the file had already been deleted over a year ago in #376.

Since the file no longer exists, the ignore should just be removed.
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.

Dropping Python 2 support
6 participants