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

Sort By Length Doesn't really sort by length #14136

Closed
scarere opened this issue Nov 6, 2024 · 1 comment
Closed

Sort By Length Doesn't really sort by length #14136

scarere opened this issue Nov 6, 2024 · 1 comment
Labels
isort Related to import sorting

Comments

@scarere
Copy link

scarere commented Nov 6, 2024

Not sure if this issue should be raised with isort or ruff, but since ruff is what I'm using raising it here. With the following ruff.toml file

fix = true
extend-select = ["I"]
extend-safe-fixes = ["I"]

# Add NOTE to default task tags
task-tags = ["TODO", "FIXME", "XXX", "NOTE"]

[lint.pycodestyle]
# Ignore overlong comments if they start with a task tag
ignore-overlong-task-comments = true

[lint.isort]
length-sort = true

ruff sorts my imports as follows

import os
import contextlib
from typing import List, Tuple
from logging import INFO
from os.path import join, isdir
from pathlib import Path

import yaml
import torch
import nnunetv2
from tqdm import tqdm
from nnunetv2.paths import nnUNet_raw
from flwr.common.logger import log
from nnunetv2.ensembling.ensemble import ensemble_folders
from nnunetv2.utilities.find_class_by_name import recursive_find_python_class
from nnunetv2.inference.predict_from_raw_data import nnUNetPredictor
from nnunetv2.utilities.plans_handling.plans_handler import PlansManager
from nnunetv2.utilities.label_handling.label_handling import (
    determine_num_input_channels,
)

It is sorting the import x imports seperately from the from x import y imports. Additionally, within the from imports, it is sorting based only on the line length of the module, not the entire line length. I tried setting force-sort-within-sections=true but this seems to override the length-sort and sort alphabetically as seen below.

import os
from typing import List, Tuple
from logging import INFO
from os.path import join, isdir
from pathlib import Path
import contextlib

from tqdm import tqdm
import yaml
import torch
import nnunetv2
from nnunetv2.paths import nnUNet_raw
from flwr.common.logger import log
from nnunetv2.ensembling.ensemble import ensemble_folders
from nnunetv2.utilities.find_class_by_name import recursive_find_python_class
from nnunetv2.inference.predict_from_raw_data import nnUNetPredictor
from nnunetv2.utilities.plans_handling.plans_handler import PlansManager
from nnunetv2.utilities.label_handling.label_handling import (
    determine_num_input_channels,
)

My expected output for length-sort=true and force-sort-within-sections=true would be

import os
import contextlib
from logging import INFO
from pathlib import Path
from typing import List, Tuple
from os.path import join, isdir

import yaml
import torch
import nnunetv2
from tqdm import tqdm
from flwr.common.logger import log
from nnunetv2.paths import nnUNet_raw
from nnunetv2.ensembling.ensemble import ensemble_folders
from nnunetv2.utilities.label_handling.label_handling import (
    determine_num_input_channels,
)
from nnunetv2.inference.predict_from_raw_data import nnUNetPredictor
from nnunetv2.utilities.plans_handling.plans_handler import PlansManager
from nnunetv2.utilities.find_class_by_name import recursive_find_python_class

To take this one step further, my actual ideal sort order would support some 'overriding rules' such as keeping imports from the same package together, and having imports that span multiple lines at the bottom. I don't know how complicated this would be to implement but it would be great if we could have sorting rules and order them based on priority. Something such as:

import os
import contextlib
from logging import INFO
from pathlib import Path
from typing import List, Tuple
from os.path import join, isdir

import yaml
import torch
import nnunetv2
from tqdm import tqdm
from flwr.common.logger import log
from nnunetv2.paths import nnUNet_raw
from nnunetv2.ensembling.ensemble import ensemble_folders
from nnunetv2.inference.predict_from_raw_data import nnUNetPredictor
from nnunetv2.utilities.plans_handling.plans_handler import PlansManager
from nnunetv2.utilities.find_class_by_name import recursive_find_python_class
from nnunetv2.utilities.label_handling.label_handling import (
    determine_num_input_channels,
)

I could create a custom section for all the nnunet imports to keep them grouped together, but the other problems remain. Is there a way for me to get the expected or desired behaviour, if not would this be a hard feature to add?

@AlexWaygood AlexWaygood added the isort Related to import sorting label Nov 6, 2024
@charliermarsh
Copy link
Member

This seems to match isort's behavior, so I'd prefer not to change it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
isort Related to import sorting
Projects
None yet
Development

No branches or pull requests

3 participants