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

Outdated information about black incompatibilities #890

Open
uhbif19 opened this issue Oct 14, 2019 · 15 comments
Open

Outdated information about black incompatibilities #890

uhbif19 opened this issue Oct 14, 2019 · 15 comments
Assignees
Labels
bug Something isn't working documentation Docs related task Hacktoberfest Hactoberfest fun! help wanted Extra attention is needed

Comments

@uhbif19
Copy link
Contributor

uhbif19 commented Oct 14, 2019

Bug report

What's wrong

Documentation states that project is is not compatible with black. As far, as I know, some points are not relevant now.

for some reasons black uses " that almost no one uses in the python world

There is option for skipping this check now: psf/black#118 (comment)

Line length. Violating rules by 10%-15% is not ok. You either violate them or not. black violates line-length rules.

Black now supports changing line length too. In our setup it has not problems with flake8.

There is still issue with trailing commas, I guess. While this does not trigger flake8 itself.

How is that should be

Sentence And there’s no configuration to fix it! must be somehow corrected, for example by adding instructions for making black work with project.

System information

Not relevant.

@uhbif19 uhbif19 added the bug Something isn't working label Oct 14, 2019
@sobolevn
Copy link
Member

sobolevn commented Oct 14, 2019

@uhbif19 thanks for the clarification! Do you want to send a PR with black tool for the official support? See how we support autopep8 here: https://github.com/wemake-services/wemake-python-styleguide/blob/master/Makefile#L7

@sobolevn sobolevn added documentation Docs related task help wanted Extra attention is needed labels Oct 14, 2019
@uhbif19
Copy link
Contributor Author

uhbif19 commented Oct 14, 2019

I am not sure what does official support means here. Does this Makefile task used for linting this repo?

I can make PR if waiting couple of weeks is okay.

Also looks like black is now leaving trailing commas, if python36 support is enabled, if I understand objective in your doc correcly. Black doc says:

Black will add trailing commas to expressions that are split by comma where each element is on its own line. This includes function signatures.

Unnecessary trailing commas are removed if an expression fits in one line. ... That doesn’t make diffs any larger.

https://black.readthedocs.io/en/stable/the_black_code_style.html?highlight=flake8#trailing-commas

@sobolevn
Copy link
Member

Yeap, official support in this context means that we check that all our code is compatible with the autoformatter we support. We guarantee it by adding this check inside the CI. And since we support python3.6+ we are also fine with the trailing comma feature.

Several weeks are totally fine, no pressure here.

@sobolevn
Copy link
Member

sobolevn commented Nov 4, 2019

Hi @uhbif19, how's it going? Do you need any help on this one?

@uhbif19
Copy link
Contributor Author

uhbif19 commented Nov 6, 2019

Have not touched that issue, so far. I plan to see it this weekend.

@sobolevn
Copy link
Member

sobolevn commented Nov 6, 2019

Thanks a lot! 👍

@sobolevn sobolevn added this to the Version 0.13 milestone Nov 6, 2019
@sobolevn sobolevn added the Hacktoberfest Hactoberfest fun! label Nov 6, 2019
@sobolevn sobolevn removed this from the Version 0.13 milestone Nov 12, 2019
@uhbif19
Copy link
Contributor Author

uhbif19 commented Nov 20, 2019

Done steps

If I understand correctly to add autopep8 analog in make lint, would be equivalent
to adding black --check .. For this codebase must be formated with black first.

I tried to format codebase with configured black, and this leads to huge
changes. Diff stat is: 303 files changed, 6739 insertions(+), 9085 deletions(-).

Then I check, if reformated codebase matches flake8 checks.
Here is stats of found violations (that, as far as I understand, it's worth considering incompatibilities):

Number of errors - Code
871 - C812: missing trailing comma
3   - C815: missing trailing comma in Python 3.5+
8   - C816: missing trailing comma in Python 3.6+
12  - D202: No blank lines allowed after function docstring
3   - E203: whitespace before ':'
34  - W503: line break before binary operator
6   - WPS221: Found line with high Jones Complexity: 17
36  - WPS317: Found incorrect multi-line parameters
2   - WPS348: Found a line that starts with a dot

Description of incompatibilities

C812 - appears in case when all arguments are on the same line, but brackets are on surrounding lines (checked 5 examples). Example of incompatible code, formatted with black:

def test_false_positives_are_ignored(
    code, assert_errors, parse_tokens, default_options
):

D202 appears only in case, when local function comes right after docstring (i checked about 5 examples of error). Example of incompatible code, formatted with black:

@pytest.fixture()
def positive_number_wrapper():
    """Fixture to return positive numbers with explicit ``+``."""

    def factory(template: str) -> str:
        return '+{0}'.format(template)

    return factory

E203 is known incompability betweeen flake8 and black. They claim that is
issue on flake8 side: psf/black#315 (comment)

W503 is known incompability betweeen black and pycodestyle. Black claims that W503 is against PEP8. pycodestyle docs says that W503 is disabled by default, because "they are not rules unanimously accepted, and PEP 8 does not enforce them". psf/black#52

WPS317 in all, but one, cases is encountered in comples @pytest.mark.parametrize. Probably somehow related with multiline lists/tuples in arguments. Example formatted with black:

@pytest.mark.parametrize(
    ('args', 'statement'),
    [
        ('self', '""""""'),
        ('self', 'return 1'),
        ('self', 'return Useless.function()'),
        ('self', 'return Useless().function()'),
        ('self', 'return Useless()().function()'),
        ('this', 'return super().function()'),
    ],
)

WPS348 - looks like black has different strategy on such cases https://black.readthedocs.io/en/stable/the_black_code_style.html#call-chains

@sobolevn
Copy link
Member

That's exactly what we can use in the docs about incompatibilities!

Great job! Thanks a lot. 👍

@DonaldTsang
Copy link

According to @ferdnyc at psf/black#1196 (comment)

It strikes me that https://wemake-python-stylegui.de/en/latest/pages/usage/integrations/auto-formatters.html#black is out of date, because according to the Black docs (emphasis mine):...

@sobolevn
Copy link
Member

@DonaldTsang thanks! Are you willing to send a PR with the fix?

@DonaldTsang
Copy link

DonaldTsang commented Dec 29, 2019

I would love to edit the text to reflect what the people have said regarding the attempt to make black compatible with WPS.

Also another note:

You could use the double-quote-string-fixer pre-commit hook.

See https://github.com/pre-commit/pre-commit-hooks

@sobolevn
Copy link
Member

Awesome, thanks!

@uhbif19
Copy link
Contributor Author

uhbif19 commented May 10, 2020

@sobolevn I am not sure what now, doc PR is required? If so, can you say what are requirements for this doc changes?

@sobolevn
Copy link
Member

@uhbif19

I am not sure what now, doc PR is required?

Yes, I would say so.

If so, can you say what are requirements for this doc changes?

I guess that #890 (comment) is enough for now.

@DonaldTsang
Copy link

DonaldTsang commented May 10, 2020

@sobolevn @uhbif19 editing text with https://hackmd.io/3vzJNofVQ_G0aS_QWwvLQA
Also I am reading #890 (comment) to see if some of those problems can be fixed.

@uhbif19 uhbif19 removed their assignment Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Docs related task Hacktoberfest Hactoberfest fun! help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants