Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider not wrapping a single import when the line is too long #3324

Open
yilei opened this issue Oct 10, 2022 · 5 comments
Open

Consider not wrapping a single import when the line is too long #3324

yilei opened this issue Oct 10, 2022 · 5 comments
Labels
T: style What do we want Blackened code to look like?

Comments

@yilei
Copy link
Contributor

yilei commented Oct 10, 2022

Describe the style change

When an import line is too long, and it only has a single imported symbol, it's more readable to NOT wrap the line.

Examples in the current Black style

from company.organization.products.my_awesome_learning_system.backend.handlers import (
    register,
)
from company.organization.products.my_awesome_learning_system.common import logging
from company.organization.products.my_awesome_learning_system.utils import (
    authentication,
)

Desired style

from company.organization.products.my_awesome_learning_system.backend.handlers import register
from company.organization.products.my_awesome_learning_system.common import logging
from company.organization.products.my_awesome_learning_system.utils import authentication

Additional context

The current behavior works very well in the majority cases. If the import line has more than one imports, it should continue wrapping them:

from company.organization.products.my_awesome_learning_system.backend.handlers import (
    enroll,
    login,
    register,
)

But for code bases that follow the Google Python Style Guide to only import modules/packages, never individual functions/classes from a module (it won't import multiple symbols in the same line, except from typing), Black can make the imports very awkward.

@yilei yilei added the T: style What do we want Blackened code to look like? label Oct 10, 2022
@Anupam-USP
Copy link

Hey, I understood the issue, but don't know how to proceed on this right now. Can you please help me to initialize this??

@yilei
Copy link
Contributor Author

yilei commented Oct 14, 2022

@Anupam-USP This is a style change proposal, so the first step is to have buy-in from Black maintainers that this is a good change.

@felix-hilden
Copy link
Collaborator

Thanks for the suggestion 👍 I'm a bit ambivalent:

Your suggestion is good for the reduced number of extra lines. It is also much better when the imports share module paths. But if the line is really long, it could be beneficial to see the final imported name on its own line so you don't have to horizontal scroll.

To have an adversarial example:

from really.long_package_name.with_submodules.that_should_probably.be_refactored_to.a_common_top_level_api.before_publishing_anything.to_pypi import this_function
from a_different_package.that_does_not_share.submodules_with_the_previous.so_you_still_need_to.read_the_path import another_function
from another_goddamn_package.that_does_not_know.how_to_manage_comes.this_module_that_is.slightly_longer_than.previous_so_your_eyes.dont_know_where.to_look import ThisClass

# VS

from really.long_package_name.with_submodules.that_should_probably.be_refactored_to.a_common_top_level_api.before_publishing_anything.to_pypi import (
    this_function
)
from a_different_package.that_does_not_share.submodules_with_the_previous.so_you_still_need_to.read_the_path import (
    another_function
)
from another_goddamn_package.that_does_not_know.how_to_manage_comes.this_module_that_is.slightly_longer_than.previous_so_your_eyes.dont_know_where.to_look import (
    ThisClass
)

Just playing the devil's advocate here. This is an unlikely scenario and I don't really feel strongly 😄

@yilei
Copy link
Contributor Author

yilei commented Oct 16, 2022

Yeah this is a relatively unlikely scenario if you look at the overall picture. When you import almost all packages on PyPI, you don't end up such long import paths.

But when a codebase is impacted, it can be pretty bad. The long import paths usually come from internal code e.g. in a mono repo. For us, such long import paths are almost everywhere.

One thing do note that since imports are sorted, it's much, much more common for adjacent imports to share module paths, than your example. Hence I think we should prioritize the relatively more common scenario and have them not wrapped.

I'll send a PR once I find myself some time.

@nameer
Copy link

nameer commented Feb 23, 2024

I find the current one more readable and less likely to break the line length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

4 participants