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

[isort] Improve import formatting configuration #4960

Closed
akihironitta opened this issue Dec 3, 2020 · 8 comments
Closed

[isort] Improve import formatting configuration #4960

akihironitta opened this issue Dec 3, 2020 · 8 comments
Labels
feature Is an improvement or enhancement help wanted Open to be worked on
Milestone

Comments

@akihironitta
Copy link
Contributor

🚀 Feature

Although #4242 have recently introduced isort to check and apply import formatting, I find the import order somewhat confusing, so I'd like to suggest to add the following two options to isort config pyproject.toml.

force_sort_within_sections = 'True'

# before
import torch
from pytorch_lightning import Callback
from torch.nn import functional as F

# after
from pytorch_lightning import Callback
import torch
from torch.nn import functional as F

order_by_type = 'False'

# before
from torch import Tensor, nn

# after
from torch import nn, Tensor

Since we have applied isort to only a few files as listed in #4805, I believe we can still modify the config. Let me know your thoughts on this. (This is the same issue as Lightning-Universe/lightning-bolts#422.)

@akihironitta akihironitta added feature Is an improvement or enhancement help wanted Open to be worked on labels Dec 3, 2020
@justusschock
Copy link
Member

cc @Borda

@Borda
Copy link
Member

Borda commented Dec 3, 2020

force_sort_within_sections = 'True'

not strong preference, but personally False as it is native in Pycharm

order_by_type = 'False'

agree :]

@Borda Borda added this to the 1.1.x milestone Dec 3, 2020
@akihironitta
Copy link
Contributor Author

force_sort_within_sections = 'True'

not strong preference, but personally False as it is native in Pycharm

@Borda Thank you for your response :] Shall I wait for other core contributors' comments, or should I give it a go with False as you prefer? (Not a strong opinion, but personally True is easier to read as torch imports, for example, are put together.)

@Borda
Copy link
Member

Borda commented Dec 4, 2020

@PyTorchLightning/core-contributors what do you think? @awaelchli 🐰

@awaelchli
Copy link
Contributor

I don't know, I never look at import statements since it's all automated so I have no preference.

@rohitgr7
Copy link
Contributor

rohitgr7 commented Dec 4, 2020

personally I'd prefer.

force_sort_within_sections=True
order_by_type=False

@akihironitta
Copy link
Contributor Author

@PyTorchLightning/core-contributors Any other thoughts?

@Borda
Copy link
Member

Borda commented Jan 2, 2021

Not a strong opinion, but personally True is easier to read as torch imports, for example, are put together.)

yes, but as from/import has different lengths it is not so easy to read... :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement help wanted Open to be worked on
Projects
None yet
Development

No branches or pull requests

5 participants