10000 gh-106233: Fix stacklevel in `zoneinfo.InvalidTZPathWarning` by sobolevn · Pull Request #106234 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-106233: Fix stacklevel in zoneinfo.InvalidTZPathWarning #106234

8000 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

Merged
merged 10 commits into from
Feb 6, 2024

Conversation

sobolevn
Copy link
Member
@sobolevn sobolevn commented Jun 29, 2023

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

@AlexWaygood
Copy link
Member

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

Copy link
Member
@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

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

Could you please add tests?

@sobolevn
Copy link
Member Author
sobolevn commented Feb 4, 2024

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
Copy link
Member

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
Copy link
Member Author
sobolevn commented Feb 5, 2024

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

Before:

» PYTHONTZPAT
8000
H=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.

Copy link
Member
@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Please add tests.

Comment on lines 1742 to 1745
self.assertTrue(
w.warnings[0].filename.endswith("test_zoneinfo.py"),
msg=w.warnings[0].filename,
)
Copy link
Member

Choose a reason for hiding this comment

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

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__)

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've already done that in ccdbaa6 :)

Copy link
Member
@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

And subTest() is no longer needed.

Copy link
Member
@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

@serhiy-storchaka serhiy-storchaka merged commit d7334e2 into python:main Feb 6, 2024
@serhiy-storchaka
Copy link
Member

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

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.12 only security fixes label Feb 6, 2024
@miss-islington-app
Copy link

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

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

(cherry picked from commit d7334e2)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-app
Copy link
bedevere-app bot commented Feb 6, 2024

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

@bedevere-app 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
…-106234) (GH-115081)

(cherry picked from commit d7334e2)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-bot
Copy link

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

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

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (d7334e2) or if it is a false positive.
  5. 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
Copy link
Member Author
sobolevn commented Feb 6, 2024

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.

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.

4 participants
0