Install black as a pre-commit hook #2442
Conversation
This commit adds an auto-formatter for Python: `black`, and adapts flake8 to avoid conflicts.
There was a problem hiding this comment.
Migrations are auto-generated so there's not much point enforcing an linter on them imo, I'd exclude migration files from it (as we currently do in the flake8 config file).
| @@ -0,0 +1,4 @@ | |||
| [tool.black] | |||
| line-length = 100 | |||
There was a problem hiding this comment.
Can we use 120? I feel that 100 is unduly conservative.
| 10: [1, 10, 55, 220, 705, 1902, 4455, 9240, 17205, 29050, 44803, 63460, 82885, 100110, 112035, 116304, 112035, 100110, 82885, 63460, 44803, 29050, 17205, 9240, 4455, 1902, 705, 220, 55, 10, 1], # noqa: E501 | ||
| 10: [ | ||
| 1, | ||
| 10, | ||
| 55, | ||
| 220, | ||
| 705, | ||
| 1902, | ||
| 4455, | ||
| 9240, | ||
| 17205, | ||
| 29050, | ||
| 44803, | ||
| 63460, | ||
| 82885, | ||
| 100110, | ||
| 112035, | ||
| 116304, | ||
| 112035, | ||
| 100110, | ||
| 82885, | ||
| 63460, | ||
| 44803, | ||
| 29050, | ||
| 17205, | ||
| 9240, | ||
| 4455, | ||
| 1902, | ||
| 705, | ||
| 220, | ||
| 55, | ||
| 10, | ||
| 1, | ||
| ], # noqa: E501 |
There was a problem hiding this comment.
Changes like this seem like clearly a bad idea to me. This had a # noqa ignore command for flake8, how do we do the same with black?
| 9: (16, 11, [16, 16, 16, 15, 15, 15, 15, 16, 15, 16, 15, 13, 13, 14, 14, 12, 14, 13, 13, 12, 13, 12, 12, 12, 12, 14, 12, 12, 11, 13, 12, 12, 13, 11, 12, 11, 11, 11, 11, 11, 13, 11, 13, 11, 12, 12, 12, 13, 12, 13, 12, 10, 13, 11, 10, 12, 10, 11, 12, 13, 10, 10, 10, 11, 12, 12, 11, 12, 9, 11, 11, 10, 9, 12, 10, 12, 9, 10, 10, 9, 11, 11, 9, 10, 11, 10, 10, 10, 9, 12, 10, 11, 10, 10, 11, 11, 11, 10, 10, 11, 8, 8, 9, 11, 8, 10, 10, 8, 9, 8, 9, 9, 9, 10, 9, 8, 10, 10, 7, 9, 10, 8, 10, 10, 9, 10, 10, 8, 10, 7, 7, 9, 8, 9, 9, 9, 9, 8, 8, 9, 8, 7, 8, 7, 9, 9, 6, 7, 6, 8, 9, 6, 6, 7, 5, 8, 7, 7, 6, 7, 6, 7, 6, 6, 0, 0]), # noqa: E501 | ||
| 8: (16, 8, [13, 15, 14, 13, 13, 13, 15, 14, 12, 13, 13, 10, 10, 14, 14, 11, 13, 11, 13, 11, 12, 11, 11, 10, 12, 12, 12, 11, 11, 11, 11, 10, 10, 9, 11, 9, 8, 8, 10, 10, 12, 8, 11, 8, 10, 11, 10, 10, 12, 13, 12, 10, 12, 9, 7, 9, 10, 10, 9, 10, 9, 8, 9, 10, 11, 10, 11, 12, 8, 10, 9, 10, 9, 10, 10, 9, 9, 8, 9, 9, 11, 9, 9, 8, 11, 9, 10, 8, 8, 9, 7, 8, 10, 10, 9, 10, 10, 10, 7, 9, 7, 8, 7, 10, 7, 9, 9, 8, 6, 8, 8, 6, 7, 10, 7, 8, 9, 7, 7, 9, 10, 6, 9, 7, 7, 8, 7, 8, 8, 6, 5, 8, 8, 7, 9, 9, 8, 8, 7, 9, 6, 6, 7, 7, 6, 8, 3, 7, 6, 5, 6, 6, 6, 7, 5, 7, 6, 6, 4, 4, 6, 5, 5, 5, 0, 0]), # noqa: E501 | ||
| 7: (16, 5, [11, 12, 13, 11, 10, 10, 14, 12, 9, 13, 12, 8, 7, 12, 11, 9, 11, 10, 11, 9, 9, 9, 8, 7, 11, 11, 10, 9, 9, 10, 9, 8, 8, 9, 9, 9, 7, 6, 8, 9, 9, 7, 10, 7, 9, 10, 9, 7, 12, 10, 9, 8, 10, 7, 6, 6, 8, 10, 8, 7, 8, 5, 8, 7, 10, 9, 8, 10, 7, 10, 8, 10, 9, 9, 10, 6, 9, 8, 7, 6, 9, 8, 8, 8, 8, 7, 8, 8, 8, 6, 7, 8, 8, 9, 9, 10, 9, 8, 7, 9, 7, 8, 7, 7, 6, 7, 7, 8, 6, 7, 6, 5, 4, 7, 6, 7, 9, 7, 4, 8, 10, 6, 8, 4, 5, 5, 6, 5, 6, 4, 5, 7, 8, 7, 7, 6, 8, 5, 7, 9, 6, 6, 6, 7, 3, 6, 3, 6, 5, 5, 5, 5, 6, 5, 5, 5, 6, 5, 4, 3, 5, 4, 5, 2, 0, 0]), # noqa: E501 | ||
| 6: (16, 2, [9, 9, 10, 9, 8, 8, 11, 11, 6, 10, 12, 8, 5, 10, 9, 9, 8, 8, 9, 7, 7, 8, 7, 7, 9, 11, 7, 6, 8, 7, 8, 7, 6, 7, 7, 9, 6, 6, 5, 9, 9, 7, 9, 6, 8, 8, 7, 7, 9, 7, 8, 8, 8, 7, 6, 5, 5, 8, 7, 7, 6, 5, 6, 4, 9, 9, 6, 8, 7, 10, 7, 8, 9, 8, 7, 5, 8, 7, 6, 6, 7, 8, 5, 8, 6, 5, 7, 5, 6, 3, 4, 5, 7, 8, 7, 7, 6, 6, 7, 8, 5, 8, 7, 6, 5, 5, 5, 8, 6, 5, 3, 5, 2, 4, 5, 7, 6, 6, 4, 8, 7, 5, 6, 3, 4, 4, 4, 5, 6, 4, 2, 7, 7, 7, 7, 6, 6, 4, 5, 7, 5, 6, 6, 4, 3, 4, 3, 3, 4, 3, 3, 5, 3, 5, 4, 4, 4, 2, 4, 2, 3, 4, 5, 1, 0, 0]), # noqa: E501 | ||
| 5: (16, -1, [6, 8, 7, 7, 8, 5, 9, 8, 5, 7, 9, 6, 5, 8, 6, 6, 6, 5, 6, 5, 6, 7, 4, 7, 7, 8, 6, 5, 8, 6, 6, 5, 3, 7, 4, 6, 4, 5, 4, 8, 6, 5, 8, 5, 6, 6, 4, 5, 7, 7, 7, 5, 6, 6, 4, 5, 5, 6, 4, 6, 3, 5, 6, 2, 6, 8, 5, 5, 6, 7, 5, 8, 6, 8, 7, 4, 6, 4, 5, 5, 4, 5, 3, 5, 4, 5, 5, 3, 5, 3, 2, 4, 7, 6, 6, 7, 6, 6, 4, 5, 3, 5, 5, 4, 4, 2, 3, 7, 4, 4, 2, 3, 0, 4, 3, 4, 5, 6, 4, 6, 6, 2, 3, 3, 3, 3, 4, 4, 5, 3, 2, 5, 5, 5, 4, 3, 6, 4, 4, 5, 5, 4, 6, 3, 2, 3, 1, 3, 2, 2, 2, 2, 3, 3, 4, 1, 1, 2, 3, 1, 3, 2, 4, 1, 0, 0]), # noqa: E501 | ||
| 4: (16, -4, [6, 5, 6, 5, 6, 5, 8, 7, 5, 6, 7, 3, 3, 5, 4, 6, 5, 2, 4, 5, 6, 5, 2, 4, 7, 6, 6, 3, 5, 4, 6, 3, 3, 4, 3, 5, 3, 5, 3, 5, 5, 4, 5, 4, 5, 5, 4, 5, 5, 5, 4, 3, 5, 6, 4, 5, 4, 3, 3, 5, 3, 5, 4, 2, 6, 5, 5, 3, 5, 5, 5, 6, 4, 5, 4, 4, 4, 2, 2, 5, 4, 3, 3, 4, 3, 3, 4, 3, 5, 1, 2, 3, 6, 3, 6, 5, 3, 4, 2, 4, 3, 4, 3, 2, 3, 1, 3, 4, 2, 2, 2, 3, 0, 4, 3, 3, 4, 4, 3, 5, 5, 2, 2, 2, 3, 3, 3, 1, 2, 2, 1, 3, 2, 3, 3, 3, 3, 4, 4, 2, 4, 1, 6, 1, 1, 0, 1, 2, 2, 0, 2, 2, 3, 3, 2, 1, 1, 2, 3, 1, 1, 1, 1, 0, 0, 0]), # noqa: E501 | ||
| 9: ( | ||
| 16, | ||
| 11, | ||
| [ |
| return ({ | ||
| 'list': getattr(view, 'list_permission', False), | ||
| 'create': getattr(view, 'create_permission', False), | ||
| 'retrieve': getattr(view, 'list_permission', False), | ||
| 'update': getattr(view, 'update_permission', False), | ||
| 'partial_update': getattr(view, 'update_permission', False), | ||
| 'destroy': getattr(view, 'destroy_permission', False), | ||
| 'delete_all': getattr(view, 'destroy_permission', False), | ||
| 'add_blank': getattr(view, 'create_permission', False), | ||
| 'GET': getattr(view, 'list_permission', False), | ||
| 'POST': getattr(view, 'update_permission', False), | ||
| 'PUT': getattr(view, 'update_permission', False), | ||
| 'PATCH': getattr(view, 'update_permission', False), | ||
| 'DELETE': getattr(view, 'destroy_permission', False), | ||
| }).get(getattr(view, 'action', view.request.method), False) | ||
| return ( | ||
| { | ||
| "list": getattr(view, "list_permission", False), | ||
| "create": getattr(view, "create_permission", False), | ||
| "retrieve": getattr(view, "list_permission", False), | ||
| "update": getattr(view, "update_permission", False), | ||
| "partial_update": getattr(view, "update_permission", False), | ||
| "destroy": getattr(view, "destroy_permission", False), | ||
| "delete_all": getattr(view, "destroy_permission", False), | ||
| "add_blank": getattr(view, "create_permission", False), | ||
| "GET": getattr(view, "list_permission", False), | ||
| "POST": getattr(view, "update_permission", False), | ||
| "PUT": getattr(view, "update_permission", False), | ||
| "PATCH": getattr(view, "update_permission", False), | ||
| "DELETE": getattr(view, "destroy_permission", False), | ||
| } | ||
| ).get(getattr(view, "action", view.request.method), False) |
There was a problem hiding this comment.
Is there a way to configure it to permit two opening brackets on the same line, where the same brackets are adjacent on both sides?
Previously we were using double quotation marks for human-readable strings, and single quotation marks for machine-readable strings. (Of course, a linter cannot enforce this, so just has to disable the check.) I have a slight preference for this but if both of you feel strongly about switching to double quotation marks that's fine. Expanding lines concerns me though. I've highlighted a particularly egregious example above, but I find oftentimes exploding nested brackets into many lines makes code less readable, not more readable. (Not always—in many of the changes here so far, there's not much difference.) There's probably some way to configure it to be more reasonable, though. |
I did a quick web search and I believe Black doesn't support this (psf/black#1281). I agree that it's not an improvement to have these long arrays wrapped on multiple lines; I think the easiest way to solve this is to use the |
This is a huge PR, as it's re-formatting a lot of the code. However only the 1st commit implements
black. An automatic code formatter is more user-friendly than a linter in that most problems get automatically fixed, so there'd be less friction as to "why can't I commit."Most of the reformatting is to prefer quotation marks over apostrophes for strings, and expanding lines. These commits can be excluded when bisecting/blaming past this PR, as explained in Black's docs.
I would want to move towards Ruff to replace both Black and Flake8 if it could have the fixes for long lines.