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

[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 https://github.com/FactoryBoy/factory_boy/pull/843#issuecomment-873033198.

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:

• https://github.com/FactoryBoy/factory_boy/pull/843#discussion_r662885480
• https://github.com/FactoryBoy/factory_boy/pull/843#discussion_r662883046
• https://github.com/FactoryBoy/factory_boy/pull/843#discussion_r662883400
• https://github.com/FactoryBoy/factory_boy/pull/843#discussion_r662883772

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. :tada: https://github.com/psf/black/pull/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