gh-106233: Fix stacklevel in `zoneinfo.InvalidTZPathWarning` by sobolevn · Pull Request #106234 · python/cpython

sobolevn · 2023-06-29T11:29:16Z

Given:

# ex.py
import zoneinfo

Now the warning looks like this:

PYTHONTZPATH=ex.py ./python.exe ex.py
path/to/ex.py:1: InvalidTZPathWarning: Invalid paths specified in PYTHONTZPATH environment variable. Paths should be absolute but found the following relative paths:
    ex.py
  import zoneinfo

Issue: Improve InvalidTZPathWarning warning with a stacklevel #106233

AlexWaygood · 2023-08-28T13:18:42Z

@sobolevn, could you address @serhiy-storchaka's comments from the linked issue? #106233 (comment)

serhiy-storchaka

What happens when call `zoneinfo.reset_tzpath() (which is the part of the API) directly in your code?

Could you please add tests?

sobolevn · 2024-02-04T09:08:51Z

Yes, these two calls have two different stacklevels :(
Fixing one breaks another (first is by an import, second is by .reset_tzpath()):

» PYTHONTZPATH=ex.py ./python.exe ex.py
/Users/sobolev/Desktop/cpython2/ex.py:1: InvalidTZPathWarning: Invalid paths specified in PYTHONTZPATH environment variable. Paths should be absolute but found the following relative paths:
    ex.py
  import zoneinfo; zoneinfo.reset_tzpath()
sys:1: InvalidTZPathWarning: Invalid paths specified in PYTHONTZPATH environment variable. Paths should be absolute but found the following relative paths:
    ex.py

I can overly-complicate this function by passing the stacklevel as an argument, when calling it on a module level, but is it worth it?

Something like:

TZPATH = ()
reset_tzpath(module_level=True)

serhiy-storchaka · 2024-02-04T09:24:16Z

The simplest way to solve this -- introduce _reset_tzpath() and call it in reset_tzpath() and on a module level. Add an explicit stacklevel parameter if needed.

sobolevn · 2024-02-05T10:34:13Z

Prep: echo 'import zoneinfo; zoneinfo.reset_tzpath()' > ex.py

Before:

» PYTHONTZPATH=ex.py ./python.exe ex.py
/Users/sobolev/Desktop/cpython2/Lib/zoneinfo/_tzpath.py:53: InvalidTZPathWarning: Invalid paths specified in PYTHONTZPATH environment variable. Paths should be absolute but found the following relative paths:
    ex.py
  warnings.warn(

Now it produces:

» PYTHONTZPATH=ex.py ./python.exe ex.py
/Users/sobolev/Desktop/cpython2/ex.py:1: InvalidTZPathWarning: Invalid paths specified in PYTHONTZPATH environment variable. Paths should be absolute but found the following relative paths:
    ex.py
  import zoneinfo; zoneinfo.reset_tzpath()

Note that two warnings are glued together, because they are identical.

serhiy-storchaka

Please add tests.

Lib/zoneinfo/_tzpath.py

serhiy-storchaka · 2024-02-05T13:45:43Z

Lib/test/test_zoneinfo/test_zoneinfo.py

Why not simply:

Suggested change

self.assertTrue(

w.warnings[0].filename.endswith("test_zoneinfo.py"),

msg=w.warnings[0].filename,

)

self.assertEqual(w.warnings[0].filename, __file__)

?

I've already done that in ccdbaa6 :)

Lib/test/test_zoneinfo/test_zoneinfo.py

serhiy-storchaka

And subTest() is no longer needed.

serhiy-storchaka

LGTM.

serhiy-storchaka · 2024-02-06T13:09:56Z

Thank you for your PR @sobolevn. Does it need backporting?

miss-islington-app · 2024-02-06T13:10:36Z

Thanks @sobolevn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

…honGH-106234) (cherry picked from commit d7334e2) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

bedevere-app · 2024-02-06T13:10:44Z

GH-115081 is a backport of this pull request to the 3.12 branch.

…-106234) (GH-115081) (cherry picked from commit d7334e2) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

bedevere-bot · 2024-02-06T14:57:17Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 Fedora Stable Clang 3.x has failed when building commit d7334e2.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/234/builds/5262) and take a look at the build logs.
Check if the failure is related to this commit (d7334e2) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/234/builds/5262

Summary of the results of the build (if available):

Click to see traceback logs

remote: Enumerating objects: 13, done.        
remote: Counting objects:   8% (1/12)        
remote: Counting objects:  16% (2/12)        
remote: Counting objects:  25% (3/12)        
remote: Counting objects:  33% (4/12)        
remote: Counting objects:  41% (5/12)        
remote: Counting objects:  50% (6/12)        
remote: Counting objects:  58% (7/12)        
remote: Counting objects:  66% (8/12)        
remote: Counting objects:  75% (9/12)        
remote: Counting objects:  83% (10/12)        
remote: Counting objects:  91% (11/12)        
remote: Counting objects: 100% (12/12)        
remote: Counting objects: 100% (12/12), done.        
remote: Compressing objects:  12% (1/8)        
remote: Compressing objects:  25% (2/8)        
remote: Compressing objects:  37% (3/8)        
remote: Compressing objects:  50% (4/8)        
remote: Compressing objects:  62% (5/8)        
remote: Compressing objects:  75% (6/8)        
remote: Compressing objects:  87% (7/8)        
remote: Compressing objects: 100% (8/8)        
remote: Compressing objects: 100% (8/8), done.        
remote: Total 13 (delta 4), reused 7 (delta 4), pack-reused 1        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to 'd7334e2c2012defaf7aae920d6a56689464509d1'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at d7334e2c20 gh-106233: Fix stacklevel in zoneinfo.InvalidTZPathWarning (GH-106234)
Switched to and reset branch 'main'

ar: unable to copy file 'libpython3.13d.a'; reason: No space left on device
make: *** [Makefile:1002: libpython3.13d.a] Error 1

sobolevn · 2024-02-06T15:00:31Z

Yes, I think that this is a bugfix that should be backported. Thanks a lot for your patience :)

ar: unable to copy file 'libpython3.13d.a'; reason: No space left on device
make: *** [Makefile:1002: libpython3.13d.a] Error 1

It is unrelated.

…honGH-106234)

pythongh-106233: Fix stacklevel in zoneinfo.InvalidTZPathWarning

ea9954f

sobolevn requested a review from pganssle June 29, 2023 11:29

bedevere-bot added the awaiting review label Jun 29, 2023

bedevere-bot mentioned this pull request Jun 29, 2023

Improve InvalidTZPathWarning warning with a stacklevel #106233

Closed

Merge branch 'main' into issue-106233

5d4dc18

serhiy-storchaka reviewed Jan 31, 2024

View reviewed changes

Use explicit stacklevel argument

4ce9123

serhiy-storchaka reviewed Feb 5, 2024

View reviewed changes

Lib/zoneinfo/_tzpath.py Outdated Show resolved Hide resolved

sobolevn added 3 commits February 5, 2024 14:21

Merge branch 'main' into issue-106233

7572618

Add test

303255f

Better use __file__

ccdbaa6

serhiy-storchaka reviewed Feb 5, 2024

View reviewed changes

Address review

0a2800e

serhiy-storchaka reviewed Feb 6, 2024

View reviewed changes

Lib/test/test_zoneinfo/test_zoneinfo.py Outdated Show resolved Hide resolved

Lib/test/test_zoneinfo/test_zoneinfo.py Outdated Show resolved Hide resolved

sobolevn added 2 commits February 6, 2024 13:44

Address review

2f9dbf4

Address review

62b1fb2

serhiy-storchaka reviewed Feb 6, 2024

View reviewed changes

Address review

0ab22a4

serhiy-storchaka approved these changes Feb 6, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Feb 6, 2024

serhiy-storchaka merged commit d7334e2 into python:main Feb 6, 2024

bedevere-app bot removed the awaiting merge label Feb 6, 2024

serhiy-storchaka added the needs backport to 3.12 only security fixes label Feb 6, 2024

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 6, 2024

pythongh-106233: Fix stacklevel in zoneinfo.InvalidTZPathWarning (pyt…

0a0c1e8

…honGH-106234) (cherry picked from commit d7334e2) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

bedevere-app bot removed the needs backport to 3.12 only security fixes label Feb 6, 2024

serhiy-storchaka pushed a commit that referenced this pull request Feb 6, 2024

[3.12] gh-106233: Fix stacklevel in zoneinfo.InvalidTZPathWarning (GH…

7974f71

…-106234) (GH-115081) (cherry picked from commit d7334e2) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this pull request Feb 14, 2024

pythongh-106233: Fix stacklevel in zoneinfo.InvalidTZPathWarning (pyt…

f6b8e2b

…honGH-106234)

-                    self.assertTrue(
-                        w.warnings[0].filename.endswith("test_zoneinfo.py"),
-                        msg=w.warnings[0].filename,
-                    )
+                    self.assertEqual(w.warnings[0].filename, __file__)

Uh oh!

Conversation

sobolevn commented Jun 29, 2023 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Aug 28, 2023

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Feb 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka commented Feb 4, 2024

Uh oh!

sobolevn commented Feb 5, 2024

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serhiy-storchaka Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

sobolevn Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Feb 6, 2024

Uh oh!

miss-islington-app bot commented Feb 6, 2024

Uh oh!

bedevere-app bot commented Feb 6, 2024

Uh oh!

bedevere-bot commented Feb 6, 2024

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

sobolevn commented Feb 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sobolevn commented Jun 29, 2023 •

edited by bedevere-bot

Loading

sobolevn commented Feb 4, 2024 •

edited

Loading