Skip to content
This repository was archived by the owner on Nov 23, 2017. It is now read-only.

Fix create_unix_server to support Path-like objects (PEP 519) #462

Closed
wants to merge 1 commit into from

Conversation

1st1
Copy link
Member

@1st1 1st1 commented Nov 12, 2016

One of the recent commits broke support of PEP 519. This PR fixes loop.create_unix_server to support Path-like objects + a regression test.

cc @brettcannon

_fspath = os.fspath
except AttributeError:
# Python 3.5 or earlier
_fspath = lambda path: path

Choose a reason for hiding this comment

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

Maybe support pathlib.Path for Python 3.5 also?
It could be implemented by isinstance() check.

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 don't think it's a good idea, PEP 519 is 3.6 only.

Copy link
Member

Choose a reason for hiding this comment

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

I think the key point is that unless a library implementing a path-representing object restricts itself to 3.6 or newer, it could be useful in 3.5 code. But I wouldn't use isinstance() as that's too restrictive if you want to bother supporting __fspath__. All you really need to be good enough for a os.fspath() fall-back is:

def _fspath(path):
  if hasattr(path, '__fspath__'):
    return path.__class__.__fspath__(path)
  else:
    return path

os.fspath() does more checks to verify that only str and bytes are returned, but that's the gist of it. Otherwise just do what Yury did.

Choose a reason for hiding this comment

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

ok

@1st1
Copy link
Member Author

1st1 commented Nov 15, 2016

Pushed in fff05d4

@1st1 1st1 closed this Nov 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants