Skip to content

Merge stdlib/{2,3}/os/path.pyi #1150

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 2 commits into from
Apr 10, 2017
Merged

Merge stdlib/{2,3}/os/path.pyi #1150

merged 2 commits into from
Apr 10, 2017

Conversation

euresti
Copy link
Contributor

@euresti euresti commented Apr 7, 2017

To be renamed into stdlib/2and3/os/path.pyi later.

Also fixes #50

To be renamed into stdlib/2and3/os/path.pyi later.

Also fixes python#50
if sys.version_info >= (3, 5):
def commonpath(paths: Sequence[AnyStr]) -> AnyStr: ...
if sys.version_info >= (3,):
# NOTE: Empty List[bytes] results in '' (str) => fall back to Any return type.
Copy link
Member

Choose a reason for hiding this comment

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

We could handle this with an overload instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to describe the type of an empty list?

Copy link
Member

Choose a reason for hiding this comment

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

You're right, we can't.

# NOTE: Empty List[bytes] results in '' (str) => fall back to Any return type.
def commonprefix(list: List[AnyStr]) -> Any: ...
else:
def commonprefix(list: List[AnyStr]) -> AnyStr: ...
Copy link
Member

Choose a reason for hiding this comment

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

Mixed lists actually work fine:

In [4]: os.path.commonprefix([u'hello', b'hello'])
Out[4]: u'hello'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the python2 version (because python3 does not let you mix types) it looks like the return type is the type of the min element of the list, https://github.com/python/cpython/blob/2.7/Lib/genericpath.py#L76

So maybe I can do an overload like this:

@overload
def commonprefix(list: List[AnyStr]) -> AnyStr: ...
@overload
def commonprefix(list: List[Union[bytes], Union[Text]]) -> Union[bytes, Text]

But then you have the dreaded Union in a return value.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean List[Union[bytes, Text]] in the second overload?

I think we can just do def commonprefix(list: Sequence[Union[bytes, Text]]) -> Any: ... (not a List so we don't get hit by invariance).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I didn't want to use Sequence[Union[bytes, Text]] because in python3 it's not supported.

)

_T = TypeVar('_T')
_PathType = Union[bytes, Text]
Copy link
Member

Choose a reason for hiding this comment

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

or os.PathLike

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look at that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looked into this I think it need more work. PathLike is 3.6 only and I imagine some methods support it. But I'm not sure this is the change to work on that.

def commonpath(paths: Sequence[AnyStr]) -> AnyStr: ...

# NOTE: Empty lists results in '' (str) regardless of contained type.
# Also, in Python 2 mixed sequences of Text and bytes results in either Text or bytes
Copy link
Member

Choose a reason for hiding this comment

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

This type signature won't allow mixed lists, right? A Sequence[Union[bytes, Text]] is not a Sequence[AnyStr].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sequence[AnyStr] works in python 2 because str gets promoted to Text

And sadly I can't overload where the argument varies by what's inside the Sequence.
Basically if I do in Python 2:

@overload
def commonprefix(list: Sequence[str]) -> str: ...
@overload
def commonprefix(list: Sequence[Text]) -> Any: ...

I get this error:
error: Overloaded function signatures 1 and 2 overlap with incompatible return types

It doesn't work with List[X] or with Sequence[bytes] in python 3. I even tried introducing a _Str = TypeVar("_Str", str) and separating using that, but that doesn't work either.

So I opted for the most generic thing.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, I forgot about that promotion. Seems like this is the best we can do then. Thanks for explaining!

@JelleZijlstra JelleZijlstra merged commit 26360e8 into python:master Apr 10, 2017
@JelleZijlstra
Copy link
Member

Thanks!

@euresti euresti deleted the os_path branch April 10, 2017 02:28
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.

os.path should use AnyStr, not unicode
2 participants