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

update to latest black #833

Merged
merged 1 commit into from
May 29, 2018
Merged

update to latest black #833

merged 1 commit into from
May 29, 2018

Conversation

gaborbernat
Copy link
Member

👍 more checks and formats

@gaborbernat gaborbernat merged commit 023eef3 into master May 29, 2018
@gaborbernat gaborbernat deleted the format branch May 29, 2018 14:40
@@ -81,7 +79,7 @@ def test_path_parts(self, input, expected):
def test_on_py_path(self):
cwd_parts = _path_parts(py.path.local())
folder_parts = _path_parts(py.path.local("a/b/c"))
assert folder_parts[len(cwd_parts):] == ["a", "b", "c"]
assert folder_parts[len(cwd_parts) :] == ["a", "b", "c"]
Copy link
Member

Choose a reason for hiding this comment

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

wat?

Choose a reason for hiding this comment

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

@ambv - why is that?

Choose a reason for hiding this comment

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

reported upstream

Choose a reason for hiding this comment

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

PyCQA/pycodestyle#373 - black is right it seems

Copy link
Member Author

Choose a reason for hiding this comment

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

Just my thoughts about this seem strange at first (like many other black style formats) but it does make sense once you look into it more in depth.

Copy link
Member

Choose a reason for hiding this comment

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

Taken from the author here:

Colons in slices are implemented to the letter of PEP 8. Your disagreement here probably stems from pycodestyle mistakenly enforcing a different rule (no spaces before colons on if-statements, defs, and so on) in the slice context.

The relevant PEP 008 section says:

However, in a slice the colon acts like a binary operator, and should have equal amounts on either side (treating it as the operator with the lowest priority). In an extended slice, both colons must have the same amount of spacing applied. Exception: when a slice parameter is omitted, the space is omitted.

I don't particularly agree with the enforced style, but the interpretation does seem to fit within the bounds of PEP 008.

Copy link

Choose a reason for hiding this comment

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

@rpkilby, if you look at the "Yes"/"No" examples below this section, you will see function calls explicitly using a space separating the colon.

Copy link
Member

Choose a reason for hiding this comment

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

@ambv the examples use a space, but it doesn't seem to be a part of the recommendation. At least, that's not how the text reads to me.

Regardless, the style black enforces is compliant, I just don't happen to like that particular choice. But again, that's just my irrelevant preference.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I agree that this is what PEP8 describes and I have succesfully been unaware of it for at least a decade, although I have red PEP8 many times. Another big 👍 for @ambv for being the perfectionist he is and getting stuff like this down to the last little detail.

@@ -87,6 +87,7 @@ commands = python -m pip list --format=columns
[flake8]
max-complexity = 22
max-line-length = 99
ignore = E203, W503
Copy link
Member

Choose a reason for hiding this comment

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

is PEP8 starting to disagree with black?

Copy link

Choose a reason for hiding this comment

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

No, pycodestyle disagrees with PEP 8.

Copy link
Member

Choose a reason for hiding this comment

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

I tend to believe you here :)

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.

6 participants