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

Possible fix for 'PosixPath' object does not support the context manager protocol #474

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jdegenstein
Copy link
Contributor

per user report on python 3.13

thanks also to @snoyer

see also: https://bugs.python.org/issue39682

This is untested so far as I haven't been able to reproduce the bug myself yet.

@jdegenstein jdegenstein marked this pull request as draft January 30, 2025 03:50
@snoyer
Copy link
Contributor

snoyer commented Jan 30, 2025

Looks like this is the right bug but not the right fix.

Key points from the Discord discussion:

@jmwright
Copy link
Member

jmwright commented Feb 1, 2025

@snoyer So would we check to see if contextlib has chdir, and monkey-patch the updated chdir (that would probably live in our codebase) in if it does not? Or is there a more elegant way of doing it?

@snoyer
Copy link
Contributor

snoyer commented Feb 1, 2025

@jmwright

So would we check to see if contextlib has chdir, and monkey-patch the updated chdir (that would probably live in our codebase) in if it does not? Or is there a more elegant way of doing it?

Defining chdir if the import fail would be a bit cleaner than monkey-patching IMO, but it would still be branching based on the Python version.

try:
    from contextlib import chdir
except ImportError:
    def chdir(path): 
        # ...

It looks like chdir would be pretty simple when implemented with @contextmanager so maybe you can just use a custom impl unconditionally and not bother with the builtin one at all?

import os
from contextlib import contextmanager
from pathlib import Path


@contextmanager
def chdir(path: str | Path):
    old_path = os.getcwd()
    os.chdir(path)
    yield
    os.chdir(old_path)


with chdir("/tmp"):
    assert os.getcwd() == "/tmp"
    with chdir("/opt"):
        assert os.getcwd() == "/opt"
    assert os.getcwd() == "/tmp"

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