cookiecutter / cookiecutter-django

Cookiecutter Django is a framework for jumpstarting production-ready Django projects quickly.
https://cookiecutter-django.readthedocs.io
BSD 3-Clause "New" or "Revised" License
12.09k stars 2.89k forks source link

Allow multiple imports on one line #4880

Closed mfosterw closed 7 months ago

mfosterw commented 8 months ago

Description

Remove the following from pyproject.toml:

[tool.ruff.lint.isort]
force-single-line = true

Rationale

I've never heard of forcing every import to be on its own line, and I am concerned that doing so will lead import blocks to be unnecessarily long. I haven't heard of many arguments in favor of this style, but I am open to them if anyone disagrees.

browniebroke commented 8 months ago

I personally don't mind too much. The main argument for forcing to be on a single line is https://github.com/asottile/reorder-python-imports#why-this-style:

The style chosen by reorder-python-imports has a single aim: reduce merge conflicts.

By having a single import per line, multiple contributors can add / remove imports from a single module without resulting in a conflict.

Consider the following example which causes a merge conflict:

# developer 1
-from typing import Dict, List
+from typing import Any, Dict, List
# developer 2
-from typing import Dict, List
+from typing import Dict, List, Tuple

no conflict with the style enforced by reorder-python-imports:

+from typing import Any
 from typing import Dict
 from typing import List
+from typing import Tuple

As I said, I don't feel strongly about it.

JamesParrott commented 8 months ago

Single line imports make it far easier to read the dependencies, and understand a module's namespace I find, as well as lending themselves towards nicer diffs.

I've never heard of forcing every import to be on its own line

I haven't heard of many arguments in favor of this style,

FYI:

https://peps.python.org/pep-0008/#imports

Imports should usually be on separate lines:

Correct:

import os import sys

Wrong:

import sys, os

https://google.github.io/styleguide/pyguide.html#s3.13-imports-formatting

3.13 Imports formatting

Imports should be on separate lines; there are exceptions for typing and collections.abc imports.

E.g.:

Yes: from collections.abc import Mapping, Sequence import os import sys from typing import Any, NewType

No: import os, sys

You are perfectly free to change details like this, in a project you create from cookiecutter-django. It's your project. It's just a coding style choice.

However to make a change like this in a popular template, that goes against an established norm, and affects all its other users too, requires a much stronger argument than one person's preference.

mfosterw commented 8 months ago

Yes: from collections.abc import Mapping, Sequence import os import sys from typing import Any, NewType

No: import os, sys

This rule is not about splitting imports from different modules, but putting imports from the same module on different lines, i.e.

from typing import Any, NewType

-->

from typing import Any from typing import NewType

It is this kind of splitting that I had never heard of. I thought that this was meant to just reduce diff size, but the point about merge conflicts does actually resonate with me somewhat, but I'm curious what other people think.

foarsitter commented 8 months ago

One of the benefits of single line imports to me is the search & replace benefits. Searching for from typing import NewType is deterministic when single line imports are enforced. I'm 100% sure if that line isn't found NewType isn't imported from typing.

luzfcb commented 8 months ago

For imports of multiple items in a single import clause, I prefer

from myapp import (
    a_module,
    b_module,
    c_function
)

instead of

from myapp import a_module, b_module, c_function

I prefer this simply because it makes long-term maintenance easier for me, especially on a code base that is touched by many hands.

It's much simpler and more straightforward to visually observe git diff and code coverage visual markup using this style. The style with everything in a single line does not allow for a quick and more direct analysis when analyzing the diff of a current commit with very old commits, which may require a little more knowledge than the trivial knowledge of git, which makes life difficult for junior developers.

For the same reason, I tend to avoid using ternary expressions, because they make it difficult to quickly visually identify code coverage and code execution path via common code coverage tools.

mfosterw commented 7 months ago

I'm sold tbh (plus I see the label)