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

Add/check os.PathLike support for the tempfile module's 'dir' arguments #73633

Open
brettcannon opened this issue Feb 5, 2017 · 11 comments
Open
Labels
3.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@brettcannon
Copy link
Member

BPO 29447
Nosy @brettcannon, @serhiy-storchaka, @mlouielu, @svelankar, @csabella
PRs
  • bpo-29447: Add os.PathLike support to the tempfile module #1411
  • bpo-29447: tempfile: Add pathlike object support for dir argument #1496
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2017-02-05.05:39:42.905>
    labels = ['type-feature', 'library', '3.9']
    title = "Add/check os.PathLike support for the tempfile module's 'dir' arguments"
    updated_at = <Date 2020-07-18.03:39:52.122>
    user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2020-07-18.03:39:52.122>
    actor = 'calestyo'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2017-02-05.05:39:42.905>
    creator = 'brett.cannon'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29447
    keywords = []
    message_count = 11.0
    messages = ['287036', '292837', '292919', '293191', '293196', '293223', '293225', '293227', '293245', '293247', '344073']
    nosy_count = 6.0
    nosy_names = ['brett.cannon', 'serhiy.storchaka', 'louielu', 'svelankar', 'cheryl.sabella', 'calestyo']
    pr_nums = ['1411', '1496']
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue29447'
    versions = ['Python 3.9']

    @brettcannon
    Copy link
    Member Author

    The various classes in the tempfile module could implement os.PathLike.

    @brettcannon brettcannon added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Feb 5, 2017
    @serhiy-storchaka
    Copy link
    Member

    TemporaryDirectory and _TemporaryFileWrapper are *not* paths, as well as ordinal files are not paths. Adding __fspath__() to them looks wrong to me.

    I think this isn't what Brett meant.

    @brettcannon
    Copy link
    Member Author

    Without looking at the PR there are two ways to interpret my unfortunately vague comment. One is to say that the objects returned by tempfile code should support os.PathLike; that's what Serhiy is objecting to as those are objects representing concrete things on the file system instead of a general concept of a path (i.e. open() returns a concrete file while pathlib returns a path). You could potentially argue that TemporaryDirectory can implement os.PathLike since it does represent a path on the file system, but I see where Serhiy is coming from about how this can be viewed as inappropriate.

    The other way to interpret what I said was to make sure things like the 'dir' argument accepted path-like objects.

    What I probably meant back in February was the first interpretation, but after hearing Serhiy's argument, I agree that the second interpretation is best. (My apologies to svelankar if they implemented the first idea.)

    @brettcannon brettcannon changed the title Add os.PathLike support to the tempfile module Add os.PathLike support to the tempfile module's 'dir' arguments May 3, 2017
    @brettcannon brettcannon changed the title Add os.PathLike support to the tempfile module's 'dir' arguments Add/check os.PathLike support for the tempfile module's 'dir' arguments May 3, 2017
    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented May 7, 2017

    Since tempfile is relay on os,

    e.g. file = _os.join.path(dir, pre+name+suf),

    it can directly accept dir as PathLike type, this should need to add test case for it.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented May 7, 2017

    Regards my words, some place need to changed to support PathLike, I'll test it tomorrow.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented May 8, 2017

    @brett, do you think if given a path-like dir, it should only be treated as str, or it could be str and bytes?

    My PR is now treated path-like dir as str, not bytes.

    This will affect at this places:

    tempfile.mkdtemp(dir=pathlike.Path(''), pre=b'', suf=b'')
    

    Should it raise a TypeError (since we can't mix str and bytes), or it will convert path-like to bytes.

    @serhiy-storchaka
    Copy link
    Member

    I don't see any changes in tempfile.py. If the path-like protocol already is supported for the dir argument, no change in the documentation is needed.

    tempfile.mkdtemp(dir=pathlike.Path(''), pre=b'', suf=b'') should raise a TypeError, but add also tests for path-like objects returning bytes path.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented May 8, 2017

    Serhiy, though this no need to add versionchanged, should this need to explicit note in doc, that tempfile support os.PathLike?

    also, I didn't get "add also tests for path-like objects returning bytes path", if tempfile.mkdtemp(dir=pathlike.Path(''), pre=b'', suf=b'') will raise TypeError, then we don't need to deal with path-like objects returning bytes. because tempfile._infer_return_type will treat path-like objects as str.

    @brettcannon
    Copy link
    Member Author

    The key thing with the docs is that it doesn't say anywhere "takes a string path" or a "path as a string" or something else that suggests path-like objects don't work. If you want to clearly state that path-like objects are acceptable that is fine as well.

    As for the bytes/str parts, path-like objects that return bytes should work, but only if everything is str or bytes as passed into the function (e.g. mixing the types should not be expected to work).

    @serhiy-storchaka
    Copy link
    Member

    If tempfile doesn't have special code for supporting path-like objects, and nothing in the documentation points that path-like objects don't work, then the documentation doesn't need changes. This is just a consequence of implementing PEP-519 in low-level functions.

    If tempfile._infer_return_type will treat *all* path-like objects as str, this is a bug.

    @csabella
    Copy link
    Contributor

    The pull request attached to this issue has been closed as the repository was marked as unknown.

    This issue is now available for a new pull request.

    @csabella csabella added the 3.9 only security fixes label May 31, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    Status: No status
    Development

    No branches or pull requests

    3 participants