Skip to content

Complete dropping Python 3.7#2975

Merged
EnricoMi merged 4 commits intoPyGithub:mainfrom
khneal:feat/support_Python_3.12
Aug 2, 2024
Merged

Complete dropping Python 3.7#2975
EnricoMi merged 4 commits intoPyGithub:mainfrom
khneal:feat/support_Python_3.12

Conversation

@khneal
Copy link
Contributor

@khneal khneal commented May 20, 2024

Python 3.12 has been GA for 6 months. Python 3.7 is EOL.

Based on #2434 and #2332

completed_at = datetime(2023, 2, 17, 16, 4, 52, tzinfo=timezone.utc)
self.assertEqual(self.job.completed_at, completed_at)
self.assertEqual(self.job.name, "test (Python 3.7)")
self.assertEqual(self.job.name, "test (Python 3.8)")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change breaks a unit test, so I will revert.

@khneal khneal marked this pull request as ready for review May 20, 2024 22:44
Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

Fine with me, please consider the proposed change.

@staticmethod
# replace with str.removesuffix once support for Python 3.7 is dropped
# replace with str.removesuffix once support for Python 3.8 is dropped
def remove_suffix(string: str, suffix: str) -> str:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you replace the usage of remove_suffix with str.removesuffix and remove this method?

Copy link
Contributor Author

@khneal khneal May 21, 2024

Choose a reason for hiding this comment

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

It would break in Python 3.8. The comment in the code was wrong, but it's fixed now.
str.removesuffix was added in 3.9, not 3.8.
See PEP 616 as well as Python Docs:

New in version 3.9.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are right, my bad!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries, and thank you!

@khneal khneal requested a review from EnricoMi May 21, 2024 13:07
Copy link

@raboof raboof left a comment

Choose a reason for hiding this comment

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

can confirm that 3.12 seems to work fine, so making that 'official' seems reasonable

@EnricoMi
Copy link
Collaborator

EnricoMi commented Aug 2, 2024

Fixed in #3008, but there seem to be some bits missing.

@EnricoMi EnricoMi changed the title Add support for Python 3.12, drop EOL 3.7 Complete dropping Python 3.7 Aug 2, 2024
@EnricoMi EnricoMi added this pull request to the merge queue Aug 2, 2024
Merged via the queue into PyGithub:main with commit d0e0507 Aug 2, 2024
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.

3 participants