Skip to content

style: isort #519

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.

Sign up for GitHub

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

Merged
henryiii merged 2 commits into from
Jan 6, 2021
Merged

style: isort #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
538c068
style: isort
henryiii Jan 5, 2021
749201a
style: pre-commit run -a
henryiii Jan 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ repos:
- id: mixed-line-ending
- id: trailing-whitespace

- repo: https://github.com/PyCQA/isort
rev: 5.7.0
hooks:
- id: isort

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.790
hooks:
Expand Down
12 changes: 7 additions & 5 deletions bin/bump_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#!/usr/bin/env python3

import click
from pathlib import Path
import glob
import os
import cibuildwheel
from packaging.version import Version, InvalidVersion
import subprocess
import glob
import urllib.parse
from pathlib import Path

import click
from packaging.version import InvalidVersion, Version

import cibuildwheel

config = [
# file path, version find/replace format
Expand Down
2 changes: 1 addition & 1 deletion bin/make_dependency_update_pr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#!/usr/bin/env python3

import os
import textwrap
import time
from pathlib import Path
from subprocess import run

import click
import textwrap


def shell(cmd, **kwargs):
Expand Down
9 changes: 4 additions & 5 deletions bin/projects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,16 @@

import builtins
import functools
import urllib.request
import xml.dom.minidom
from datetime import datetime
from io import StringIO
from typing import Dict, Any, List, Optional, TextIO
from pathlib import Path
from typing import Any, Dict, List, Optional, TextIO

import click
import yaml
from github import Github
import urllib.request
import xml.dom.minidom
from pathlib import Path


ICONS = (
"appveyor",
Expand Down
9 changes: 5 additions & 4 deletions bin/run_example_ci_configs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
#!/usr/bin/env python3

import os
from pathlib import Path
import shutil
from subprocess import run
import sys
import textwrap
import time
import click
from glob import glob
from collections import namedtuple
from glob import glob
from pathlib import Path
from subprocess import run
from urllib.parse import quote

import click


def shell(cmd, **kwargs):
return run([cmd], shell=True, **kwargs)
Expand Down
2 changes: 1 addition & 1 deletion bin/sample_build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#!/usr/bin/env python3

import argparse
import os
import subprocess
import sys
import argparse
import tempfile
from pathlib import Path

Expand Down
6 changes: 1 addition & 5 deletions cibuildwheel/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,13 @@
import traceback
from configparser import ConfigParser
from pathlib import Path

from typing import Any, Dict, List, Optional, Set, overload

import cibuildwheel
import cibuildwheel.linux
import cibuildwheel.macos
import cibuildwheel.windows
from cibuildwheel.environment import (
EnvironmentParseError,
parse_environment,
)
from cibuildwheel.environment import EnvironmentParseError, parse_environment
from cibuildwheel.util import (
Architecture,
BuildOptions,
Expand Down
2 changes: 1 addition & 1 deletion cibuildwheel/docker_container.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import sys
import uuid
from pathlib import Path, PurePath
from typing import cast, IO, Dict, List, Optional, Sequence, Type
from types import TracebackType
from typing import IO, Dict, List, Optional, Sequence, Type, cast

from .typing import PathOrStr, PopenBytes

Expand Down
4 changes: 2 additions & 2 deletions cibuildwheel/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import bashlex

from typing import Dict, List, Mapping, Optional

import bashlex

from . import bashlex_eval


Expand Down
11 changes: 8 additions & 3 deletions cibuildwheel/linux.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@

from .docker_container import DockerContainer
from .logger import log
from .typing import PathOrStr
from .util import (
Architecture, BuildOptions, BuildSelector, NonPlatformWheelError,
allowed_architectures_check, get_build_verbosity_extra_flags, prepare_command,
Architecture,
BuildOptions,
BuildSelector,
NonPlatformWheelError,
allowed_architectures_check,
get_build_verbosity_extra_flags,
prepare_command,
)
from .typing import PathOrStr


class PythonConfiguration(NamedTuple):
Expand Down
2 changes: 1 addition & 1 deletion cibuildwheel/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import re
import sys
import time
from typing import Optional, Union, AnyStr, IO
from typing import IO, AnyStr, Optional, Union

from cibuildwheel.util import CIProvider, detect_ci_provider

Expand Down
14 changes: 11 additions & 3 deletions cibuildwheel/macos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@

from .environment import ParsedEnvironment
from .logger import log
from .util import (BuildOptions, BuildSelector, NonPlatformWheelError,
download, get_build_verbosity_extra_flags, get_pip_script,
install_certifi_script, prepare_command, allowed_architectures_check)
from .typing import PathOrStr
from .util import (
BuildOptions,
BuildSelector,
NonPlatformWheelError,
allowed_architectures_check,
download,
get_build_verbosity_extra_flags,
get_pip_script,
install_certifi_script,
prepare_command,
)


def call(args: Sequence[PathOrStr], env: Optional[Dict[str, str]] = None, cwd: Optional[str] = None, shell: bool = False) -> int:
Expand Down
2 changes: 1 addition & 1 deletion cibuildwheel/resources/get-pip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import os.path
import pkgutil
import shutil
import sys
import struct
import sys
import tempfile

# Useful for very coarse version differentiation.
Expand Down
1 change: 1 addition & 0 deletions cibuildwheel/resources/install_certifi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def main():
"-E", "-s", "-m", "pip", "install", "--upgrade", "certifi"])

import certifi

# change working directory to the default SSL directory
os.chdir(openssl_dir)
relpath_to_certifi_cafile = os.path.relpath(certifi.where())
Expand Down
3 changes: 1 addition & 2 deletions cibuildwheel/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from typing import Union, TYPE_CHECKING
import os
import subprocess

from typing import TYPE_CHECKING, Union

if TYPE_CHECKING:
PopenBytes = subprocess.Popen[bytes]
Expand Down
2 changes: 1 addition & 1 deletion cibuildwheel/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import functools
import os
import platform as platform_module
import re
import ssl
import sys
import functools
import textwrap
import urllib.request
from enum import Enum
Expand Down
14 changes: 11 additions & 3 deletions cibuildwheel/windows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,18 @@

from .environment import ParsedEnvironment
from .logger import log
from .util import (Architecture, BuildOptions, BuildSelector, NonPlatformWheelError,
download, get_build_verbosity_extra_flags, get_pip_script,
prepare_command, allowed_architectures_check)
from .typing import PathOrStr
from .util import (
Architecture,
BuildOptions,
BuildSelector,
NonPlatformWheelError,
allowed_architectures_check,
download,
get_build_verbosity_extra_flags,
get_pip_script,
prepare_command,
)

IS_RUNNING_ON_AZURE = Path('C:\\hostedtoolcache').exists()
IS_RUNNING_ON_TRAVIS = os.environ.get('TRAVIS_OS_NAME') == 'windows'
Expand Down
4 changes: 4 additions & 0 deletions setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,7 @@ ignore_missing_imports = True

[mypy-bashlex.*]
ignore_missing_imports = True

[tool:isort]
profile=black
multi_line_output=3
2 changes: 2 additions & 0 deletions test/test_0_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import platform
import textwrap

import pytest

from cibuildwheel.logger import Logger

from . import test_projects, utils

basic_project = test_projects.new_c_project(
Expand Down
6 changes: 3 additions & 3 deletions test/test_before_all.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import pytest
import subprocess
import textwrap

from . import utils
from . import test_projects
import pytest

from . import test_projects, utils
Copy link
Contributor Author

@henryiii henryiii Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly confusing, actually, the thing after a from x import ideally shouldn't be a module; if you have an __init__.py and it contains a test_projects or utils object, you'll get that instead of the module. But it's a bit special for . imports, since you are required to use from with a relative import.

I believe you are guaranteed to have the test directory as the first on the path in PyTest, so this could be just import test_projects, etc? Also, we could from .test_projects import new_c_project, etc.

Copy link
Contributor Author

@henryiii henryiii Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the exiting ones are written this way, so I'd probably leave it for now, and eventually rewrite them properly (see test_0_basic.py, for example).

Copy link
Member

@YannickJadoul YannickJadoul Jan 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I first wanted to suggest import .test_projects, but apparently that only works in the from ... import ... construct.

Copy link
Contributor

@joerick joerick Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the thing after a from x import ideally shouldn't be a module

Not sure I agree, e.g. from numpy import linalg from django import models from tensorflow import keras... also I'd rather rely on the __init__.py file being empty than rely on the value of sys.path. I expect tools like mypy or language servers to be able to read the code better without relying on a sys.path set by pytest too.

Copy link
Contributor Author

@henryiii henryiii Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from numpy import linalg is an often used shortcut for import numpy.linalg as linalg, but it is confusing - a reader can't tell if linalg is a package or an object. We are stuck with it, but I wish the language wasn't ambiguous. Ideally, relative imports probably should import objects, from .test_projects import new_c_project.

Relative imports also don't work if you don't execute it as a package; python test_before_all.py will fail. (ImportError: attempted relative import with no known parent package) (PyTest handles this correctly).

Copy link
Contributor Author

@henryiii henryiii Jan 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it matters, from a users' point of view.

If you try to rewrite it, you have to either run it interactively or look up the source. For example:

from a import b
b.c()
# May or may not be written as
from a.b import c
c()

And, if you need to look up the source, is there an a/b.py or an a/__init__.py file with a b in it, or is it a file? (I often teach Python, this sort of thing can be a pain point...)

Copy link
Contributor Author

@henryiii henryiii Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use relative imports a lot, but they have downsides, especially for new users who do not know how to set up a proper environment. They have to be in a loaded package; it's not only "in the same directory". So python x.py inside a package fails if there are relative imports in x; I think python outer/x.py fails too (I could be wrong). If you stick a if __name__ == "__main__" in it for standalone runs or tests, it fails.

Copy link
Contributor Author

@henryiii henryiii Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also use absolute imports a lot...

Copy link
Contributor Author

@henryiii henryiii Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do know someone who will not do any import except import a.b.c, and always uses fully qualified names. That's it. No shortening. It drive me crazy seeing numpy.stuff sometimes. :)

Copy link
Member

@YannickJadoul YannickJadoul Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW (given we're on this tangent anyway?), I tend to not like from module import thing either. Often, module.thing is not so much longer to write, should be auto-completed, and we're not trying to save bytes in code anyway? But yes, numpy to np just becomes a habit, after a while :-)


project_with_before_build_asserts = test_projects.new_c_project(
setup_py_add=textwrap.dedent(r'''
Expand Down
6 changes: 3 additions & 3 deletions test/test_before_build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import pytest
import subprocess
import textwrap

from . import utils
from . import test_projects
import pytest

from . import test_projects, utils

project_with_before_build_asserts = test_projects.new_c_project(
setup_py_add=textwrap.dedent(r'''
Expand Down
3 changes: 1 addition & 2 deletions test/test_before_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from . import test_projects
from . import utils
from . import test_projects, utils

before_test_project = test_projects.new_c_project()
before_test_project.files['test/spam_test.py'] = r'''
Expand Down
3 changes: 1 addition & 2 deletions test/test_build_skip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import textwrap

from . import utils
from . import test_projects
from . import test_projects, utils

project_with_skip_asserts = test_projects.new_c_project(
setup_py_add=textwrap.dedent(r'''
Expand Down
8 changes: 4 additions & 4 deletions test/test_dependency_versions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import re
import pytest
import textwrap
import cibuildwheel.util

from . import utils
from . import test_projects
import pytest

import cibuildwheel.util

from . import test_projects, utils

project_with_expected_version_checks = test_projects.new_c_project(
setup_py_add=textwrap.dedent(r'''
Expand Down
3 changes: 1 addition & 2 deletions test/test_docker_images.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@

import pytest

from . import utils
from . import test_projects
from . import test_projects, utils

dockcross_only_project = test_projects.new_c_project(
setup_py_add=textwrap.dedent(r'''
Expand Down
4 changes: 2 additions & 2 deletions test/test_emulation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import subprocess

import pytest
from . import utils
from . import test_projects

from . import test_projects, utils

project_with_a_test = test_projects.new_c_project()

Expand Down
6 changes: 3 additions & 3 deletions test/test_environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import os
import pytest
import subprocess
import textwrap
from . import utils
from . import test_projects

import pytest

from . import test_projects, utils

project_with_environment_asserts = test_projects.new_c_project(
setup_py_add=textwrap.dedent(r'''
Expand Down
3 changes: 1 addition & 2 deletions test/test_manylinuxXXXX_only.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@

import pytest

from . import utils
from . import test_projects
from . import test_projects, utils

# TODO: specify these at runtime according to manylinux_image
project_with_manylinux_symbols = test_projects.new_c_project(
Expand Down
6 changes: 3 additions & 3 deletions test/test_pep518.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import textwrap
from . import test_projects
from . import utils
import os
import textwrap

from . import test_projects, utils

basic_project = test_projects.new_c_project(
setup_py_add=textwrap.dedent(
Expand Down
4 changes: 1 addition & 3 deletions test/test_projects/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
from pathlib import Path
from typing import Any, Dict, Union

import jinja2

from typing import Union, Dict, Any


FilesDict = Dict[str, Union[str, jinja2.Template]]
TemplateContext = Dict[str, Any]

Expand Down
Loading