Skip to content

Created pyi for SimpleHTTPServer [Python 2.7] #1186

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 4 commits into from
Apr 25, 2017

Conversation

Solumin
Copy link
Contributor

@Solumin Solumin commented Apr 21, 2017

No description provided.

server_version = ... # type: str
def do_GET(self) -> None: ...
def do_HEAD(self) -> None: ...
def send_head(self) -> Optional[IO[AnyStr]]: ...
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean Optional[IO[str]]? Makes sense.

def do_HEAD(self) -> None: ...
def send_head(self) -> Optional[IO[AnyStr]]: ...
def list_directory(self, path: str) -> Optional[StringIO]: ...
def translate_path(self, path: str) -> str: ...
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 this one actually could use 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.

Done.

def do_GET(self) -> None: ...
def do_HEAD(self) -> None: ...
def send_head(self) -> Optional[IO[AnyStr]]: ...
def list_directory(self, path: str) -> Optional[StringIO]: ...
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 argument for this one can also be unicode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to AnyStr, because os.listdir takes AnyStr.

def list_directory(self, path: str) -> Optional[StringIO]: ...
def translate_path(self, path: str) -> str: ...
def copyfile(self, source: IO[AnyStr], outputfile: IO[AnyStr]): ...
def guess_type(self, path: str) -> str: ...
Copy link
Member

Choose a reason for hiding this comment

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

The argument (but not the return type) can also be unicode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So Union[str, unicode] instead of AnyStr for path? Or does it not make any difference here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Union[str, unicode] based on #871

def do_HEAD(self) -> None: ...
def send_head(self) -> Optional[IO[str]]: ...
def list_directory(self, path: AnyStr) -> Optional[StringIO]: ...
def translate_path(self, path: AnyStr) -> str: ...
Copy link
Member

Choose a reason for hiding this comment

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

The return value should also be AnyStr. In general, a single AnyStr doesn't make sense, because the point of the AnyStr type variable is to constrain two types to be the same (either str or unicode).

As far as I'm aware, using only AnyStr in a signature is meaningful only when it's the type argument to a generic class like List. In that case, List[AnyStr] means the list must either have only str or only unicode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. I'm still wrapping my head around TypeVars, I guess. I keep forgetting that it binds to only one type of string and isn't the same as a Union.

def do_GET(self) -> None: ...
def do_HEAD(self) -> None: ...
def send_head(self) -> Optional[IO[str]]: ...
def list_directory(self, path: AnyStr) -> Optional[StringIO]: ...
Copy link
Member

Choose a reason for hiding this comment

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

This argument should be a Union. listdir takes AnyStr because its return value depends on the type passed in, but that's not true for this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

Petition to rename AnyStr to ExactlyOneStr?

Copy link
Member

Choose a reason for hiding this comment

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

I kind of agree. I'm not sure AnyStr should have been added to typing, since it's frequently misunderstood and it's really much more common that you want Union[str, unicode].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TypeVar(str, unicode) certainly has its uses where you don't want to mix string types together, so it's really just a naming issue. Or it doesn't need to have been a default part of typing, since it's not actually something special.

I'd rather have AnyStr = Union[str, unicode] and EitherStr = TypeVar(str, unicode). Kind of hard to distill "exactly one of str or unicode" into a pithy variable name, unfortunately.

- `list_directory` takes `Union...` instead of `AnyStr`
- `translate_path -> AnyStr`
@JelleZijlstra JelleZijlstra merged commit f71ea30 into python:master Apr 25, 2017
@JelleZijlstra
Copy link
Member

Thanks!

@Solumin
Copy link
Contributor Author

Solumin commented Apr 25, 2017

Thank you for the extremely helpful reviews!

@Solumin Solumin deleted the simplehttpserver branch April 25, 2017 23:19
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.

2 participants