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

gh-112451: Prohibit subclassing of datetime.timezone. #114190

Merged
merged 5 commits into from
Jan 26, 2024

Conversation

felixxm
Copy link
Contributor

@felixxm felixxm commented Jan 17, 2024

This is consistent with C-extension datetime.timezone.

@felixxm felixxm changed the title gh-112451: Make datetime.timezone not acceptable as a base class. gh-112451: Prohibit subclassing of datetime.timezone. Jan 17, 2024
@erlend-aasland
Copy link
Contributor

This will need a deprecation period as per PEP-387.

@felixxm
Copy link
Contributor Author

felixxm commented Jan 17, 2024

This will need a deprecation period as per PEP-387.

Great, my first deprecation in Python 🎉 WIP

@felixxm felixxm changed the title gh-112451: Prohibit subclassing of datetime.timezone. gh-112451: Deprecate subclassing of pure-Python datetime.timezone. Jan 17, 2024
@felixxm
Copy link
Contributor Author

felixxm commented Jan 17, 2024

Updated 👍

@pganssle
Copy link
Member

This will need a deprecation period as per PEP-387.

I guess it isn't the worst thing in the world to have this deprecation period, but I think for PEP 399 compliance, especially when the feature is available only in the pure-Python module (which is not commonly used), there's a case to be made that this doesn't need a deprecation period.

I'm sure I'll eat my words when some PyPy-only user shows up and tells us they have been subclassing datetime.timezone, but I feel like it wouldn't be a huge deal if we just removed this functionality in the pure Python module? I don't want this question to block this PR indefinitely, so if it takes a long time to get a response we should go ahead with the warning, but I opened an issue with the steering council for clarification on this question.

@erlend-aasland
Copy link
Contributor

Oh, TIL PEP-399; thanks, Paul. I'm always fascinated on the various ways people depend on very specific behaviour. Thanks for opening an issue over at the SC tracker.

@felixxm felixxm changed the title gh-112451: Deprecate subclassing of pure-Python datetime.timezone. gh-112451: Prohibit subclassing of datetime.timezone. Jan 24, 2024
This is consistent with C-extension datetime.timezone.
@felixxm
Copy link
Contributor Author

felixxm commented Jan 24, 2024

I reverted the deprecation per the Steering Council decision.

Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for bearing through the uncertainty @felixxm !

@erlend-aasland
Copy link
Contributor

Yes, thanks @felixxm! And thanks a lot to @pganssle and @serhiy-storchaka for your guidance 🙏

@felixxm
Copy link
Contributor Author

felixxm commented Jan 26, 2024

Thanks y'all for help and reviews 🥇

@erlend-aasland erlend-aasland merged commit 456e274 into python:main Jan 26, 2024
29 checks passed
@felixxm felixxm deleted the gh-112451-cannot branch January 26, 2024 08:41
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…14190)

This is consistent with C-extension datetime.timezone.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…14190)

This is consistent with C-extension datetime.timezone.
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