Format code with Black

[francoisfreitag]

Black is a very popular Python code formatter. It ensures a consistent
coding style across the library. It is opinionated and not configurable,
so that projects adopting Black are similar.

The main advantage a code formatter is to avoid concerns about coding
style. Many text editors have an integration for Black and can be
configured to format on save.

The isort profile has been set to "black" for compatibility.
https://pycqa.github.io/isort/docs/configuration/black_compatibility/

https://github.com/psf/black

TODOS

• Apply https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html#avoiding-ruining-git-blame
• Move isort and flake8 config to pyproject.toml (later/earlier commit)
• Investigate moving build options to pyproject.toml

[francoisfreitag]

My plan is to apply https://github.com/psf/black#migrating-your-code-style-without-ruining-git-blame if this is integrated.

[rbarrois]

I strongly object to black's default choices; I take great care to ensure that my code is readable by humans, which requires heuristics that black has been unable to understand.

I understand its use for projects with lots of contributors; however, let's be honest: the core of factory_boy is quite complex, and contributions far and few between — most of the work consists on contributions consists of guiding the contributor into understanding the core concepts and considering all use cases for their code; style has never in my experience been an issue with pull requests.

So, that's a strong -1 for me.

[francoisfreitag]

I agree that style is not a major hurdle to overcome for contributors, but a consistent coding style increases the source code readability. It also takes coding style out of the discussion entirely, which is the main benefit IMO.

Black certainly cannot outperform a human on a given piece of code. However, to maintain a consistent coding-style across the project is particularly hard for humans, especially when multiple humans are contributing the code.

I did a quick read-through of the source code after formatting it, to find cases where Black changed the layout in a way that decreased readability. A common example is when data structures have leading commas, Black spreads them on multiple lines:

# Before Black
class MyFactory(DjangoModelFactory):
    class Meta:
        model = "myapp.Author"
        django_get_or_create = ["foo", "bar", "baz", "other",]

# After Black
class MyFactory(DjangoModelFactory):
    class Meta:
        model = "myapp.Author"
        django_get_or_create = [
            "foo",
            "bar",
            "baz",
            "other",
        ]

When I spotted such changes, I fixed them (by removing the trailing comma, to let Black know the structure should not be expanded). If you find places that can be improved upon, I’m happy to take a look and discuss them.

IMO, just because Black does not always outperform humans is not a reason to outright reject it.

I don’t feel strongly about adding it in, but I believe it is valuable to the project:

1. Increase overall consistency, which benefits readability
2. Leaves style out of the discussion for contributors
3. Follows other projects in the Python ecosystem in using the source code formatter

[rbarrois]

After one hour of my life spent reviewing this code, this is my feedback:

• Switching from ' to " is noisy. I don't really care, but it might have been easier to review is split in a dedicated commit;
• black provides some improvements in a few places, I agree with the pattern for multi-line interpolation (putting the % (...) on the second line instead of having the % at the end of the first line);
• It isn't able to put trailing commas in place, which is incompatible with my understanding (and factory_boy's flavour) of PEP8;
• It makes logging calls much nosier, whereas they should be kept compact in order to avoid breaking the reading flow of the program.

Unless we can address at least those two points (trailing commas, logging calls), I don't see the point of going through the hassle of migrating to black.

[rbarrois]

I prefer this to be split across lines, so that the condition (if k in post_overrides) stays easy to spot

[francoisfreitag]

Suggested change that would also solve this comment. #877

[rbarrois]

Same here

[francoisfreitag]

Suggested change that would also solve this comment. #877

[rbarrois]

With all the added indent levels, I find the final structure much nosier.

[jdufresne]

@rbarrois I think much of the feedback could be resolved using "magic trailing commas":
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma

but it might have been easier to review is split in a dedicated commit;

The Black --skip-string-normalization might help split this into multiple commits. First fix all non-string formatting, second apply magic trailing commas where desired, finally apply string normalization.

[francoisfreitag]

Thanks for the feedback.

I split the commit following the Jon’s suggestion #843 (comment).

It makes logging calls much nosier, whereas they should be kept compact in order to avoid breaking the reading flow of the program.

A way to keep logging on a single line for most cases would be to increase the line length. That would also solve:

• Format code with Black #843 (comment)
• Format code with Black #843 (comment)
• Format code with Black #843 (comment)
• Format code with Black #843 (comment)

I’ll give that a try.

[francoisfreitag]

Pushed a commit that sets the line length to 120.

If we go that route, the isort and flake8 configuration could be moved to the pyproject.toml as a later step.

[francoisfreitag]

Black will probably be moving out of beta during 2022. 🎉
psf/black#2529

[kingbuzzman]

The link https://github.com/psf/black#migrating-your-code-style-without-ruining-git-blame has changed to https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html?highlight=blame#avoiding-ruining-git-blame

time is unforgiving :P