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

PEP 519 support in the stdlib #71369

Closed
ethanfurman opened this issue Jun 2, 2016 · 21 comments
Closed

PEP 519 support in the stdlib #71369

ethanfurman opened this issue Jun 2, 2016 · 21 comments
Labels
type-feature A feature request or enhancement

Comments

@ethanfurman
Copy link
Member

BPO 27182
Nosy @brettcannon, @ethanfurman, @serhiy-storchaka, @JelleZijlstra
Dependencies
  • bpo-26027: Support Path objects in the posix module
  • bpo-26667: Update importlib to accept pathlib.Path objects
  • bpo-27184: Support path objects in the ntpath module
  • bpo-27186: add os.fspath()
  • Files
  • issue27182-open.patch: patch adding fspath support to builtins.open
  • issue27182-path_type.patch
  • test_fspath.py
  • os.diff: Support for os.walk() and os.fwalk() (depends on path_converter)
  • 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 = <Date 2020-11-06.19:37:09.804>
    created_at = <Date 2016-06-02.17:16:29.052>
    labels = ['type-feature']
    title = 'PEP 519 support in the stdlib'
    updated_at = <Date 2020-11-06.19:37:09.804>
    user = 'https://github.com/ethanfurman'

    bugs.python.org fields:

    activity = <Date 2020-11-06.19:37:09.804>
    actor = 'brett.cannon'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-11-06.19:37:09.804>
    closer = 'brett.cannon'
    components = []
    creation = <Date 2016-06-02.17:16:29.052>
    creator = 'ethan.furman'
    dependencies = ['26027', '26667', '27184', '27186']
    files = ['43156', '43201', '43213', '44091']
    hgrepos = []
    issue_num = 27182
    keywords = ['patch']
    message_count = 21.0
    messages = ['266891', '266892', '266893', '266897', '266901', '266908', '266953', '267123', '267131', '267132', '267280', '267282', '267287', '267335', '267340', '267457', '268065', '272063', '272559', '273737', '274712']
    nosy_count = 5.0
    nosy_names = ['brett.cannon', 'ethan.furman', 'python-dev', 'serhiy.storchaka', 'JelleZijlstra']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue27182'
    versions = ['Python 3.6']

    @ethanfurman
    Copy link
    Member Author

    Meta issue to track adding PEP-519 support in the various stdlib modules.

    @ethanfurman
    Copy link
    Member Author

    posix module: bpo-26027

    @ethanfurman
    Copy link
    Member Author

    importlib: bpo-26667

    @ethanfurman
    Copy link
    Member Author

    nt module: bpo-27184

    @ethanfurman ethanfurman added type-bug An unexpected behavior, bug, or error type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jun 2, 2016
    @ethanfurman
    Copy link
    Member Author

    os.fspath(): bpo-27186

    @serhiy-storchaka
    Copy link
    Member

    Isn't the nt module just an alias of the posix module?

    @ethanfurman
    Copy link
    Member Author

    Nope.

    There is a posixpath.py and an ntpath.py, and they are not the same.

    @JelleZijlstra
    Copy link
    Member

    FYI, I'm working on a patch for builtins.open to call PyOS_FSPath.

    @JelleZijlstra
    Copy link
    Member

    This patch makes the Python and C versions of open()/io.open() support the fspath protocol.

    @ethanfurman
    Copy link
    Member Author

    Sorry, Serhiy, I had my module names mixed up.

    nt and posix are the same, but ntpath and posixpath are not (I may have those first two names wrong again, but hopefully you get the idea).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 4, 2016

    New changeset 00991aa5fdb5 by Ethan Furman in branch 'default':
    bpo-27182: update fsencode and fsdecode for os.path(); patch by Dusty Phillips
    https://hg.python.org/cpython/rev/00991aa5fdb5

    @JelleZijlstra
    Copy link
    Member

    In the patch that was just committed, the path_type variables are undefined (os.py lines 892 and 908). There is also no test for these error cases—we should test the cases where the path has no __fspath__ or does not return a bytes or str.

    @JelleZijlstra
    Copy link
    Member

    Attached patch fixes the undefined variable and adds additional tests.

    @ethanfurman
    Copy link
    Member Author

    Currently, os.fspath will raise an exception if the thing passed in is not str/bytes/PathLike, and that error message will proclaim that str or bytes or PathLike is required; however, this is not true in cases such as Path (which doesn't allow bytes), and incomplete in cases such as os.open (which also allows ints).

    On the other hand, if the thing has a functional __fspath__ (meaning calling it doesn't raise an exception) then os.fspath will return whatever that method returns, which could be complete garbage.

    So os.fspath is being too strict, too open, and too lax all at the same time.

    Given Guido's reluctance to check the output of __fspath__(), plus the current difficulty of painless integration with existing functions, I think we should have os.fspath() only raise an exception if obj.__fspath__ exists and calling it raises an exception, otherwise we return the result of calling obj.__fspath__(), or obj if it doesn't have __fspath__.

    In case that wasn't clear, attached is a unit test that passes when the above changes are implemented.

    @JelleZijlstra
    Copy link
    Member

    If we do that, then os.* functions that accept fds would also work on objects whose __fspath__ method returns an integer. I don't think that is desirable (I was just writing a test to ensure that fspath returning an integer throws an error).

    @brettcannon
    Copy link
    Member

    Functions that only accept file descriptors should not be updated to work with __fspath__() as it will never return an int/fd.

    As for Ethan's suggestion, are you saying you want to toss the str/bytes check from os.fspath()? If so then you will need to go to python-dev and bring that up as the PEP clearly specifies that str/bytes is checked for and specifically in the order of the Python code. The thinking behind the current design is that since __fspath__() has to be explicitly implemented that people will do so properly, versus accidentally passing in some type that isn't str/bytes like the pre-PEP 519 world (i.e. trust the __fspath__() implementors to do the right thing and only protect against someone passing in something wrong from complicated code flow).

    There has been discussion about using the path.__fspath__() if hasattr(path, '__fspath__') else path idiom in os.path so that the pre-existing type-checks can do their thing instead of checking twice, although that's different from how os.fspath() works (then again, since this is all new code we could argue that going our own route in os.path is acceptable in the name of performance).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 9, 2016

    New changeset 6239673d5e1d by Brett Cannon in branch 'default':
    Issue bpo-27182: Document os.PathLike.
    https://hg.python.org/cpython/rev/6239673d5e1d

    @brettcannon brettcannon self-assigned this Jun 9, 2016
    @brettcannon
    Copy link
    Member

    Just a quick update: between the patches for issue bpo-26027 and issue bpo-26667, the necessary code to make os.path work with path-like objects is done. At this point I'm just waiting for code reviews on those patches.

    @brettcannon
    Copy link
    Member

    Here is an odd patch because I don't know where else to put it ATM that adds the remaining support in the os module/package not covered by other issues w/ patches (specifically os.walk() and os.fwalk()). I think everything else simply falls through thanks to os.path and path_converter.

    @brettcannon
    Copy link
    Member

    The os and os.path modules are now done! The means PEP-519 is finished. At this point individual modules will need to be checked to see if they do (not) support os.PathLike.

    @brettcannon brettcannon removed their assignment Aug 26, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 7, 2016

    New changeset 3417d324cbf9 by Brett Cannon in branch 'default':
    Issue bpo-27182: Add support for path-like objects to PyUnicode_FSDecoder().
    https://hg.python.org/cpython/rev/3417d324cbf9

    @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
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants