gh-72346: Added isdst deprecation warning to email.utils.localtime #91450

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":

Lib/email/utils.py

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!

Doc/whatsnew/3.11.rst

Doc/library/email.utils.rst

Lib/email/utils.py

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

Doc/library/email.utils.rst

@@ -26,6 +26,8 @@ module:

    .. 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

Doc/whatsnew/3.11.rst

@@ -853,6 +853,9 @@ Deprecated

  (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

Lib/email/utils.py

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

    """
+    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

Lib/email/utils.py

@@ -345,6 +346,8 @@ def localtime(dt=None, isdst=-1):
    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

Lib/email/utils.py

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

    """
+    if isdst != -1:
+        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():

if dt is None: dt = datetime.datetime.now() return dt.astimezone()

(Extra code for the deprecated parameter not shown :))

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

Lib/email/utils.py

@@ -29,6 +29,7 @@
 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

Doc/library/email.utils.rst

@@ -26,6 +26,8 @@ module:

    .. 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

Lib/email/utils.py

@@ -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

Lib/email/utils.py

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

    """
+    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} is deprecated and slated for removal in Python {remove},

remove=(3, 13),

)

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

Doc/library/email.utils.rst

@@ -11,7 +11,7 @@
 There are a couple of useful utilities provided in the :mod:`email.utils`
 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

Doc/whatsnew/3.11.rst

@@ -853,6 +853,9 @@ Deprecated

  (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.

Lib/email/utils.py

…into fix-pythongh-72346

Doc/whatsnew/3.12.rst

…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

Lib/email/utils.py Outdated Show resolved Hide resolved

added documentation and used _deprecated

add07f3

Removed trailing whitespace.

854ebf8

hugovk reviewed Apr 12, 2022

View reviewed changes

Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved

Doc/library/email.utils.rst Outdated Show resolved Hide resolved

Lib/email/utils.py Outdated Show resolved Hide resolved

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 Python modules in the Lib dir 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

Lib/email/utils.py Outdated Show resolved Hide resolved

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

Revert incorrect change.

9389791

Merge branch 'fix-pythongh-72346' of https://github.com/aw1231/cpython …

672b5e5

…into fix-pythongh-72346

Merge branch 'main' into fix-pythongh-72346

89eb11a

Merge branch 'fix-pythongh-72346' of https://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

Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved

alanfwilliams added 2 commits September 8, 2022 10:26

Moved whatsnew note

d0eda9d

Merge branch 'fix-pythongh-72346' of https://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:main Mar 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.

		@@ -853,6 +853,9 @@ 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))

		@@ -26,6 +26,8 @@ module:

		.. versionadded:: 3.3

		.. deprecated:: 3.11

		@@ -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} is deprecated and slated for removal in Python {remove},
+            remove=(3, 13),
+            )

Uh oh!

gh-72346: Added isdst deprecation warning to email.utils.localtime #91450

gh-72346: Added isdst deprecation warning to email.utils.localtime #91450

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!

Uh oh!

alanfwilliams commented Apr 11, 2022 •

edited by bedevere-bot

Loading

hugovk left a comment •

edited

Loading