isort and black config

[mauritsvanrees]

Several packages have an isort config. Often something like this:

[isort]
# black compatible Plone isort rules:
profile = black
force_alphabetical_sort = True
force_single_line = True
lines_after_imports = 2

But ever since isort has gotten profiles, like the black used above, it has also had a plone profile. See the current profiles. The black and plone profiles are defined like this:

black = {
    "multi_line_output": 3,
    "include_trailing_comma": True,
    "force_grid_wrap": 0,
    "use_parentheses": True,
    "ensure_newline_before_comments": True,
    "line_length": 88,
}
plone = {
    "force_alphabetical_sort": True,
    "force_single_line": True,
    "lines_after_imports": 2,
    "line_length": 200,
}

The author seems to have gotten the config out of the Plone style guide.

Compared with the top most config, the plone profile has line length 200, which we want to be 88 for standard black compatibility.
And the black config has some extra options that influence how to handle a too long import. For a moment I thought this might not matter, because we have force_single_line = True, but it does matter. Otherwise you get difference like this when running both black and isort:

-        from plone.app.content.browser.contents.properties import (  # noqa
-            PropertiesActionView,
-        )
+        from plone.app.content.browser.contents.properties import \
+            PropertiesActionView  # noqa

Neither the black nor the plone profile has changed since two years, so they are pretty stable.

Shall we ask to update the plone profile in isort? Then we can simply use this isort config:

[isort]
profile = plone

It seems best to copy the black settings and add our own sauce on top:

plone = {
    # Keep the first part the same as black please:
    "multi_line_output": 3,
    "include_trailing_comma": True,
    "force_grid_wrap": 0,
    "use_parentheses": True,
    "ensure_newline_before_comments": True,
    "line_length": 88,
    # The next part has extra config on top of black:
    "force_alphabetical_sort": True,
    "force_single_line": True,
    "lines_after_imports": 2,
}

Does that sound good?

See also the isort options documentation.
And PLIP #2754.

[ale-rt]

That sounds really beautiful :)

[mauritsvanrees]

Anyone else want to comment?
It would not be much use if we change this in isort, and then have to revert it in a new release.

[mauritsvanrees]

@jensens You did some isort commits in Plone recently. Do you agree with the above?

[gforcada]

Yes, that sounds like a really good settings 👍🏾 and the final result, these only 2 lines are pure beauty 🤩

[jensens]

sounds good. so we need to change our styleguide to use black and this isort config too?

[MrTango]

the only thing would be, when ever something changes in black, there is a chance that our profile get's outdated, where the black profile will be update to date or sure.
What is the reason to use a plone profile here?
The black profile works fine for me.

[mauritsvanrees]

the only thing would be, when ever something changes in black, there is a chance that our profile get's outdated, where the black profile will be update to date or sure.

I have made an isort PR where I copy the black settings on-the-fly and add our extra sauce on top.

What is the reason to use a plone profile here?
The black profile works fine for me.

In the Plone style guide we have advocated the use of these three extra settings for years:

    "force_alphabetical_sort": True,
    "force_single_line": True,
    "lines_after_imports": 2,

Most important is the first one, which says: we do not care about what is first/second/third party code, just sort everything alphabetically please.

[1letter]

i like black with the three options like @mauritsvanrees says, that makes sense.

[mauritsvanrees]

My PR to isort has been merged. Now we need to wait for a release. Could be a while, because the CI is broken (unrelated to my PR), and I can imagine the author prefers to fix that first.

[idgserpro]

@mauritsvanrees

https://github.com/PyCQA/isort/blob/main/CHANGELOG.md#5110-december-12-2022

Update plone profile: copy of black, plus three settings. (#1926)

[mauritsvanrees]

@idgserpro Ah, thanks for letting us know. So isort 5.11.0 is out and contains the fix; and 5.11.3 is current latest. That took long to get in a release, but it is there now, so we can be happy and change our configs to simply this:

[isort]
profile = plone

[gforcada]

I'm having a look at how https://github.com/zopefoundation/meta works to create a similar tool for plone, first to add black/isort/pyupgrade to keep formatting consistent, then we can move to tests, coverage and what not :)

Is anyone interested, are there any efforts towards such scenario that I'm not aware of? 🤔

[mauritsvanrees]

Definitely interested in something like that. There is no effort yet, as far as I know.

Well, there is the plone/code-analysis-action which can do linting. Used for example in plone.dexterity.

[gforcada]

Oh, thanks for the pointer and the ready example 👍🏾 I will have a look during Christmas vacations when I want to get some time off family 😆

[gforcada]

We are already using black/isort and plenty of other tooling 🎉