gh-72346: Added isdst deprecation warning to email.utils.localtime by alanfwilliams · Pull Request #91450 · python/cpython

alanfwilliams · 2022-04-11T18:31:47Z

This adds a deprecation warning to the isdst parameter in email.utils.localtime. This is my first contribution so any feedback is welcome.

Issue: Deprecate isdst argument in email.utils.localtime #72346

alanfwilliams · 2022-04-11T18:40:57Z

I'm not sure why it says I haven't signed the CLA. I did just link it with my github account so it may take a moment to refresh I assume.

zware · 2022-04-11T19:50:06Z

Closing and re-opening to re-trigger the CLA check.

hugovk

A good start, thanks!

It would need some documentation updating too.

Put a .. deprecated:: 3.11 under the function in email.utils.rst. For example, have a look at:

And also list it on "What’s New In Python 3.11":

alanfwilliams · 2022-04-12T17:35:10Z

@hugovk I've added the edits you've requested. Thanks for the quick review.

hugovk

Thanks for the update!

alanfwilliams · 2022-04-13T14:54:10Z

@hugovk I've completed the changes you requested.

hugovk

Thanks! Just a couple of minor style things!

hugovk · 2022-04-13T19:11:56Z

 .. versionadded:: 3.3

+ .. deprecated:: 3.11
+ Use of the ``isdst`` argument is deprecated.


I just noticed the style for parameters is italics (for example see the docstring):
Suggested change
Use of the ``isdst`` argument is deprecated.
Use of the *isdst* argument is deprecated.

hugovk · 2022-04-13T19:12:09Z


 (Contributed by Brett Cannon in :issue:`47061`.)

+* Deprecated the ``isdst`` parameter in :func:`email.utils.localtime`.


Suggested change
* Deprecated the ``isdst`` parameter in :func:`email.utils.localtime`.
* Deprecated the *isdst* parameter in :func:`email.utils.localtime`.

hugovk · 2022-04-13T19:13:46Z


 """
+ if isdst != -1:
+ warnings._deprecated("The isdst parameter to localtime", remove=(3,13))


A little nit :)
Suggested change
warnings._deprecated("The isdst parameter to localtime", remove=(3,13))
warnings._deprecated("The isdst parameter to localtime", remove=(3,13))

hugovk

Thanks, looks good! Hopefully a core dev can take a look soon and trigger the CI.

cc @abalkin, @bitdancer, @Mariatta

alanfwilliams · 2022-04-26T15:56:18Z

Hi, just wanted to check if there was anything I could do to move this along.

zware

This is a great start! A few things to fix up, though.

There will also need to be test changes for this, both to test that the deprecation warning is emitted as expected, and to prevent it from escaping where we don't want it.

Also, note that I'm not a maintainer of the email package, so these are just general suggestions to hopefully make this quicker and easier for one of them to accept :)

zware · 2022-04-13T19:49:04Z

 to divine whether summer time is in effect for the specified time.

 """
+ if isdst != -1:


We need to change the default value here to detect when someone is passing in -1 explicitly. None is fine as the new default, or you could add _ignored = object() just before this function and use _ignored as the new default.

zware · 2022-04-13T19:49:55Z

+ warnings._deprecated("The isdst parameter to localtime", remove=(3,13))
 if dt is None:
 return datetime.datetime.now(datetime.timezone.utc).astimezone()
 if dt.tzinfo is not None:


As mentioned in the issue, this whole function can be reduced to just a thin wrapper around datetime.astimezone():
ifdtisNone: dt=datetime.datetime.now() returndt.astimezone()
(Extra code for the deprecated parameter not shown :))

zware · 2022-04-13T19:52:01Z

 import socket
 import datetime
 import urllib.parse
+import warnings


Since this should generally not be necessary unless someone is passing in a value for isdst, it can be imported where it's used.

zware · 2022-04-13T19:53:26Z


 .. versionadded:: 3.3

+ .. deprecated:: 3.11


Since we're specifying in the code that it will be removed in 3.13, we can use the deprecated-removed directive here to also document when it will disappear.

zware · 2022-04-13T19:55:23Z

@@ -345,6 +346,8 @@ def localtime(dt=None, isdst=-1):
 to divine whether summer time is in effect for the specified time.


The docstring should be updated to remove everything about isdst, and replace it with a simple note that the parameter is ignored. Similar in the documentation.

zware · 2022-04-16T17:14:40Z


 """
+ if isdst != -1:
+ warnings._deprecated("The isdst parameter to localtime", remove=(3, 13))


Unfortunately, the easy route here makes for an ugly message: 'The isdst argument to localtime' is deprecated .... We need to specify our own message to make it prettier:
Suggested change
warnings._deprecated("The isdst parameter to localtime", remove=(3, 13))
warnings._deprecated(
"The 'isdst' parameter to 'localtime'",
message='{name}isdeprecatedandslatedforremovalinPython {remove},
remove=(3, 13),
)

zware · 2022-04-26T18:21:30Z

 module:

-.. function:: localtime(dt=None)
+.. function:: localtime(dt=None, isdst=-1)


Since it wasn't documented before and is being removed, we probably don't want to add it now :). The deprecation note can mention that it was previously undocumented.

zware · 2022-04-26T18:24:53Z

 (Contributed by Brett Cannon in :issue:`47061`.)

+* Deprecated the *isdst* parameter in :func:`email.utils.localtime`.
+ (Contributed by Alan Williams in :issue:`72346`.)


:issue:`72346` will direct to https://bugs.python.org/issue72346, which doesn't exist :). This should now be :gh:`72346` , IIUC.

bedevere-bot · 2022-04-26T18:36:13Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

…into fix-pythongh-72346

alanfwilliams · 2022-09-08T18:49:01Z

Although it seems that the Azure test isn't working properly (says it passed in the output, but is marked as failed), I have made the requested changes; please review again.

bedevere-bot · 2022-09-08T18:49:05Z

Thanks for making the requested changes!

@zware: please review the changes made to this pull request.

alanfwilliams · 2022-10-21T15:09:32Z

Hey @zware, any chance you can take a look at this?

zware · 2023-03-20T00:20:02Z

5 months later, yes! LGTM, sorry for the delay.

…ime (pythonGH-91450)

Added isdst deprecation warning
dadf5e2

alanfwilliams requested a review from a team as a code owner April 11, 2022 18:31

bedevere-bot added the awaiting review label Apr 11, 2022

📜🤖 Added by blurb_it.
77414f9

This comment was marked as outdated.
Sign in to view

zware closed this Apr 11, 2022

zware reopened this Apr 11, 2022

zware linked an issue Apr 12, 2022 that may be closed by this pull request
Deprecate isdst argument in email.utils.localtime #72346
Closed

hugovk reviewed Apr 12, 2022
View reviewed changes

Comment threadLib/email/utils.py Outdated

added documentation and used _deprecated
add07f3

Removed trailing whitespace.
854ebf8

hugovk reviewed Apr 12, 2022
View reviewed changes

Comment threadDoc/whatsnew/3.11.rst Outdated
Comment threadDoc/library/email.utils.rst Outdated
Comment threadLib/email/utils.py Outdated

added newline, moved whatsnew, reformatted warning
1b9a60f

hugovk reviewed Apr 13, 2022
View reviewed changes

formatting corrections
6784b42

hugovk approved these changes Apr 13, 2022
View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Apr 13, 2022

hugovk added topic-email stdlib Standard Library Python modules in the Lib/ directory 3.11 only security fixes labels Apr 13, 2022

zware requested changes Apr 26, 2022
View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Apr 26, 2022

ezio-melotti reviewed Aug 30, 2022
View reviewed changes

Comment threadLib/email/utils.py Outdated

ezio-melottiand others added 6 commits August 30, 2022 23:23

Revert incorrect change.
9389791

Merge branch 'fix-pythongh-72346' ofhttps://github.com/aw1231/cpython…
672b5e5
…into fix-pythongh-72346

Merge branch 'main' into fix-pythongh-72346
89eb11a

Merge branch 'fix-pythongh-72346' ofhttps://github.com/aw1231/cpython…
c2b4c94
…into fix-pythongh-72346

Modified existing tests/added test for deprecation
31a4c46

Merge branch 'main' into fix-pythongh-72346
54596ef

hugovk reviewed Sep 8, 2022
View reviewed changes

Comment threadDoc/whatsnew/3.12.rst Outdated

alanfwilliams added 2 commits September 8, 2022 10:26

Moved whatsnew note
d0eda9d

Merge branch 'fix-pythongh-72346' ofhttps://github.com/aw1231/cpython…
ecc3bff
…into fix-pythongh-72346

bedevere-bot added awaiting change review and removed awaiting changes labels Sep 8, 2022

bedevere-bot requested a review from zware September 8, 2022 18:49

alanfwilliams added 3 commits October 7, 2022 13:38

Merge branch 'main' into fix-pythongh-72346
8151b5a

Adjust formatting
4c3413a

Merge branch 'main' into fix-pythongh-72346
7ff165e

zware approved these changes Mar 20, 2023
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 20, 2023

zware merged commit 5e6661b into python:mainMar 20, 2023

bedevere-bot removed the awaiting merge label Mar 20, 2023

zware added 3.12 only security fixes and removed 3.11 only security fixes labels Mar 20, 2023

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
pythongh-72346: Added isdst deprecation warning to email.utils.localt…
7dab359
…ime (pythonGH-91450)

warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
pythongh-72346: Added isdst deprecation warning to email.utils.localt…
a715639
…ime (pythonGH-91450)

hugovk mentioned this pull request May 8, 2024
Remove isdst parameter from email.utils.localtime#118798
Closed

	Use of the ``isdst`` argument is deprecated.
	Use of the isdst argument is deprecated.


		(Contributed by Brett Cannon in :issue:`47061`.)

		* Deprecated the ``isdst`` parameter in :func:`email.utils.localtime`.

	* Deprecated the ``isdst`` parameter in :func:`email.utils.localtime`.
	* Deprecated the isdst parameter in :func:`email.utils.localtime`.

	warnings._deprecated("The isdst parameter to localtime", remove=(3,13))
	warnings._deprecated("The isdst parameter to localtime", remove=(3,13))

		@@ -345,6 +346,8 @@ def localtime(dt=None, isdst=-1):
		to divine whether summer time is in effect for the specified time.

-warnings._deprecated("The isdst parameter to localtime", remove=(3, 13))
+warnings._deprecated(
+"The 'isdst' parameter to 'localtime'",
+message='{name}isdeprecatedandslatedforremovalinPython {remove},
+remove=(3, 13),
+ )

Uh oh!

Uh oh!

Conversation

alanfwilliams commented Apr 11, 2022• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

alanfwilliams commented Apr 11, 2022

Uh oh!

zware commented Apr 11, 2022

Uh oh!

hugovk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alanfwilliams commented Apr 12, 2022

Uh oh!

hugovk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alanfwilliams commented Apr 13, 2022

Uh oh!

hugovk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugovk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanfwilliams commented Apr 26, 2022

Uh oh!

zware left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Apr 26, 2022

Uh oh!

Uh oh!

Uh oh!

alanfwilliams commented Sep 8, 2022

Uh oh!

bedevere-bot commented Sep 8, 2022

Uh oh!

alanfwilliams commented Oct 21, 2022

Uh oh!

zware commented Mar 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alanfwilliams commented Apr 11, 2022•
edited by bedevere-bot
Loading

hugovk left a comment •
edited
Loading