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 spaces around "=" in signature repr. #1198

Merged
merged 1 commit into from
Dec 27, 2017
Merged

Add spaces around "=" in signature repr. #1198

merged 1 commit into from
Dec 27, 2017

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 27, 2017

PEP8 indicates (correctly, IMO) that when an annotation is present, the
signature should include spaces around the equal sign, i.e.

def f(x: int = 1): ...

instead of

def f(x: int=1): ...

(in the latter case the equal appears to bind to the type, not to the
argument).

pybind11 signatures always includes a type annotation so we can always
add the spaces.

PEP8 indicates (correctly, IMO) that when an annotation is present, the
signature should include spaces around the equal sign, i.e.

    def f(x: int = 1): ...

instead of

    def f(x: int=1): ...

(in the latter case the equal appears to bind to the type, not to the
argument).

pybind11 signatures always includes a type annotation so we can always
add the spaces.
@jagerman
Copy link
Member

This sounds good to me. PEP8 aside, I also agree that it looks better in most cases.

@jagerman
Copy link
Member

jagerman commented Dec 27, 2017

pybind11 signatures always includes a type annotation so we can always add the spaces.

It seems to me that for some cases (e.g. taking a py::object or py::handle argument) we'd be better off not adding the type annotation. But that's not a concern for this PR.

@jagerman jagerman merged commit 0826b3c into pybind:master Dec 27, 2017
@anntzer anntzer deleted the spaces-around-equal-in-sig branch December 27, 2017 18:05
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