Skip to content

Commit

Permalink
[Tidy First] Use isort to order/format/de-dupe python imports (#10085)
Browse files Browse the repository at this point in the history
* Add `isort` as a dev-req and pre-commit hook

The tool `isort` reorders imports to be in alphabetical order. I've
added it because our imports in most files are in random order. The lack
of order meant that:
- sometimes the same module would be imported from twice
- figuring out if a a module was already being imported from took
  longer

In the next commit I'll actually run isort to order everything. The best
part is that when developing, we don't have to put them in correct order.
Though you can if you want. However, `isort` will take care of re-ordering
things at commit time :)

* Improve isort functionality by setting initial `.isort.cfg`

Specifically we set two config values: `extend_skip_glob` and `known_first_party`.
The `extend_skip_glob` extends the default skipped paths. The defaults can be seen
here https://pycqa.github.io/isort/docs/configuration/options.html#skip. We are skipping
third party stubs because these are more so provided (I believe). We are skipping
`.github` and `scripts` as they feel out of scope and things we can be less strict with.

The `known_first_party` setting makes it so that these imports get grouped separately from
all other imports, which is useful visually of "this comes from us" vs "this comes from
someone/somewhere else".

* Add profile `black` to isort config

I was seeing some odd behavior where running pre-commit, adding the modified
files, and then running pre-commit again would result making more modifications
to some of the same files. This felt odd. You shouldn't have to run pre-commit
more multiple times for it to eventually come to a final "solution". I believe
the problem was because we are using the tool `black` to format things, but weren't
registering the black profile with `isort` this lead to some conflicting formatting
rules, and the two tools had to negotiate a few times before being both satisfied.
Registering the profile `black` with `isort` resolved this problem.

* Reorder, merge-duplicate, and format module imports using `isort`

This was done by running `pre-commit run --all`. I ran it separately from
the commit process itself because I wanted to run it against all files
instead of only changed files.

Of note, this not only reordered and formatted our imports. But we also
had 60 distinct duplicate module import paths across 50 files, which this
took care of. When I say "distinct duplicate module import paths" I mean
when `from x.y.z import` was imported more than once in a single file.
  • Loading branch information
QMalcolm authored May 3, 2024
1 parent dc744f6 commit 29b8359
Show file tree
Hide file tree
Showing 477 changed files with 2,892 additions and 2,941 deletions.
4 changes: 4 additions & 0 deletions .isort.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[settings]
profile=black
extend_skip_glob=.github/*,third-party-stubs/*,scripts/*
known_first_party=dbt,dbt_adapters,dbt_common,dbt_extractor,dbt_semantic_interface
4 changes: 4 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ repos:
exclude_types:
- "markdown"
- id: check-case-conflict
- repo: https://github.com/pycqa/isort
rev: 5.12.0
hooks:
- id: isort
- repo: https://github.com/psf/black
rev: 22.3.0
hooks:
Expand Down
59 changes: 27 additions & 32 deletions core/dbt/artifacts/resources/__init__.py
Original file line number Diff line number Diff line change
@@ -1,41 +1,40 @@
from dbt.artifacts.resources.base import BaseResource, GraphResource, FileHash, Docs
from dbt.artifacts.resources.base import BaseResource, Docs, FileHash, GraphResource
from dbt.artifacts.resources.v1.analysis import Analysis

# alias to latest resource definitions
from dbt.artifacts.resources.v1.components import (
DependsOn,
NodeVersion,
RefArgs,
HasRelationMetadata,
ParsedResourceMandatory,
ParsedResource,
ColumnInfo,
CompiledResource,
InjectedCTE,
Contract,
DeferRelation,
DependsOn,
FreshnessThreshold,
HasRelationMetadata,
InjectedCTE,
NodeVersion,
ParsedResource,
ParsedResourceMandatory,
Quoting,
RefArgs,
Time,
)
from dbt.artifacts.resources.v1.analysis import Analysis
from dbt.artifacts.resources.v1.hook import HookNode
from dbt.artifacts.resources.v1.model import Model, ModelConfig
from dbt.artifacts.resources.v1.sql_operation import SqlOperation
from dbt.artifacts.resources.v1.seed import Seed, SeedConfig
from dbt.artifacts.resources.v1.singular_test import SingularTest
from dbt.artifacts.resources.v1.generic_test import GenericTest, TestMetadata
from dbt.artifacts.resources.v1.snapshot import Snapshot, SnapshotConfig


from dbt.artifacts.resources.v1.config import (
Hook,
NodeAndTestConfig,
NodeConfig,
TestConfig,
)
from dbt.artifacts.resources.v1.documentation import Documentation
from dbt.artifacts.resources.v1.exposure import (
Exposure,
ExposureConfig,
ExposureType,
MaturityType,
)
from dbt.artifacts.resources.v1.macro import Macro, MacroDependsOn, MacroArgument
from dbt.artifacts.resources.v1.generic_test import GenericTest, TestMetadata
from dbt.artifacts.resources.v1.group import Group
from dbt.artifacts.resources.v1.hook import HookNode
from dbt.artifacts.resources.v1.macro import Macro, MacroArgument, MacroDependsOn
from dbt.artifacts.resources.v1.metric import (
ConstantPropertyInput,
ConversionTypeParams,
Expand All @@ -46,6 +45,7 @@
MetricTimeWindow,
MetricTypeParams,
)
from dbt.artifacts.resources.v1.model import Model, ModelConfig
from dbt.artifacts.resources.v1.owner import Owner
from dbt.artifacts.resources.v1.saved_query import (
Export,
Expand All @@ -55,6 +55,7 @@
SavedQueryConfig,
SavedQueryMandatory,
)
from dbt.artifacts.resources.v1.seed import Seed, SeedConfig
from dbt.artifacts.resources.v1.semantic_layer_components import (
FileSlice,
SourceFileMetadata,
Expand All @@ -74,28 +75,22 @@
SemanticModel,
SemanticModelConfig,
)

from dbt.artifacts.resources.v1.config import (
NodeAndTestConfig,
NodeConfig,
TestConfig,
Hook,
)

from dbt.artifacts.resources.v1.singular_test import SingularTest
from dbt.artifacts.resources.v1.snapshot import Snapshot, SnapshotConfig
from dbt.artifacts.resources.v1.source_definition import (
SourceConfig,
ExternalPartition,
ExternalTable,
SourceDefinition,
ParsedSourceMandatory,
SourceConfig,
SourceDefinition,
)

from dbt.artifacts.resources.v1.sql_operation import SqlOperation
from dbt.artifacts.resources.v1.unit_test_definition import (
UnitTestConfig,
UnitTestDefinition,
UnitTestFormat,
UnitTestInputFixture,
UnitTestNodeVersions,
UnitTestOutputFixture,
UnitTestOverrides,
UnitTestNodeVersions,
UnitTestFormat,
)
4 changes: 2 additions & 2 deletions core/dbt/artifacts/resources/base.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import hashlib
from dataclasses import dataclass
from dbt_common.dataclass_schema import dbtClassMixin
from typing import List, Optional
import hashlib

from dbt.artifacts.resources.types import NodeType
from dbt_common.dataclass_schema import dbtClassMixin


@dataclass
Expand Down
5 changes: 3 additions & 2 deletions core/dbt/artifacts/resources/v1/analysis.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from dbt.artifacts.resources.v1.components import CompiledResource
from typing import Literal
from dataclasses import dataclass
from typing import Literal

from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.components import CompiledResource


@dataclass
Expand Down
13 changes: 6 additions & 7 deletions core/dbt/artifacts/resources/v1/components.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import time
from dataclasses import dataclass, field
from dbt.artifacts.resources.base import GraphResource, FileHash, Docs
from dbt.artifacts.resources.types import NodeType
from datetime import timedelta
from typing import Any, Dict, List, Optional, Union

from dbt.artifacts.resources.base import Docs, FileHash, GraphResource
from dbt.artifacts.resources.types import NodeType, TimePeriod
from dbt.artifacts.resources.v1.config import NodeConfig
from dbt_common.dataclass_schema import dbtClassMixin, ExtensibleDbtClassMixin
from dbt_common.contracts.config.properties import AdditionalPropertiesMixin
from dbt_common.contracts.constraints import ColumnLevelConstraint
from typing import Dict, List, Optional, Union, Any
from datetime import timedelta
from dbt.artifacts.resources.types import TimePeriod
from dbt_common.contracts.util import Mergeable

from dbt_common.dataclass_schema import ExtensibleDbtClassMixin, dbtClassMixin

NodeVersion = Union[str, float]

Expand Down
21 changes: 9 additions & 12 deletions core/dbt/artifacts/resources/v1/config.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
import re
from dataclasses import dataclass, field
from typing import Any, Dict, List, Optional, Union

from dbt_common.dataclass_schema import dbtClassMixin, ValidationError
from typing import Optional, List, Any, Dict, Union
from mashumaro.jsonschema.annotations import Pattern
from typing_extensions import Annotated
from dataclasses import dataclass, field
from dbt_common.contracts.config.base import (
BaseConfig,
CompareBehavior,
MergeBehavior,
)
from dbt_common.contracts.config.metadata import Metadata, ShowBehavior
from dbt_common.contracts.config.materialization import OnConfigurationChangeOption

from dbt import hooks
from dbt.artifacts.resources.base import Docs
from dbt.artifacts.resources.types import ModelHookType
from dbt.artifacts.utils.validation import validate_color
from dbt import hooks
from mashumaro.jsonschema.annotations import Pattern
from dbt_common.contracts.config.base import BaseConfig, CompareBehavior, MergeBehavior
from dbt_common.contracts.config.materialization import OnConfigurationChangeOption
from dbt_common.contracts.config.metadata import Metadata, ShowBehavior
from dbt_common.dataclass_schema import ValidationError, dbtClassMixin


def list_str() -> List[str]:
Expand Down
5 changes: 3 additions & 2 deletions core/dbt/artifacts/resources/v1/exposure.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import time
from dataclasses import dataclass, field
from typing import Any, Dict, List, Literal, Optional

from dbt.artifacts.resources.base import GraphResource
from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.components import DependsOn, RefArgs
from dbt.artifacts.resources.v1.owner import Owner
from dbt_common.contracts.config.base import BaseConfig
from dbt_common.dataclass_schema import StrEnum
import time
from typing import Any, Dict, List, Literal, Optional


class ExposureType(StrEnum):
Expand Down
7 changes: 4 additions & 3 deletions core/dbt/artifacts/resources/v1/generic_test.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from dataclasses import dataclass, field
from typing import Optional, Any, Dict, Literal
from dbt_common.dataclass_schema import dbtClassMixin
from typing import Any, Dict, Literal, Optional

from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.config import TestConfig
from dbt.artifacts.resources.v1.components import CompiledResource
from dbt.artifacts.resources.v1.config import TestConfig
from dbt_common.dataclass_schema import dbtClassMixin


@dataclass
Expand Down
5 changes: 3 additions & 2 deletions core/dbt/artifacts/resources/v1/hook.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from dataclasses import dataclass
from typing import Optional, Literal
from dbt.artifacts.resources.v1.components import CompiledResource
from typing import Literal, Optional

from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.components import CompiledResource


@dataclass
Expand Down
8 changes: 4 additions & 4 deletions core/dbt/artifacts/resources/v1/macro.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from dataclasses import dataclass, field
import time
from typing import Literal, List, Dict, Optional, Any
from dataclasses import dataclass, field
from typing import Any, Dict, List, Literal, Optional

from dbt_common.dataclass_schema import dbtClassMixin
from dbt.artifacts.resources.base import BaseResource, Docs
from dbt.artifacts.resources.types import NodeType, ModelLanguage
from dbt.artifacts.resources.types import ModelLanguage, NodeType
from dbt.artifacts.resources.v1.components import MacroDependsOn
from dbt_common.dataclass_schema import dbtClassMixin


@dataclass
Expand Down
18 changes: 9 additions & 9 deletions core/dbt/artifacts/resources/v1/metric.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import time

from dataclasses import dataclass, field
from typing import Any, Dict, List, Literal, Optional

from dbt_semantic_interfaces.references import MeasureReference, MetricReference
from dbt_semantic_interfaces.type_enums import (
ConversionCalculationType,
MetricType,
TimeGranularity,
)

from dbt.artifacts.resources.base import GraphResource
from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.components import DependsOn, RefArgs
Expand All @@ -10,14 +18,6 @@
)
from dbt_common.contracts.config.base import BaseConfig, CompareBehavior, MergeBehavior
from dbt_common.dataclass_schema import dbtClassMixin
from dbt_semantic_interfaces.references import MeasureReference, MetricReference
from dbt_semantic_interfaces.type_enums import (
ConversionCalculationType,
MetricType,
TimeGranularity,
)
from typing import Any, Dict, List, Literal, Optional


"""
The following classes are dataclasses which are used to construct the Metric
Expand Down
13 changes: 9 additions & 4 deletions core/dbt/artifacts/resources/v1/model.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
from dataclasses import dataclass, field
from typing import Literal, Optional, List
from datetime import datetime
from typing import List, Literal, Optional

from dbt.artifacts.resources.types import AccessType, NodeType
from dbt.artifacts.resources.v1.components import (
CompiledResource,
DeferRelation,
NodeVersion,
)
from dbt.artifacts.resources.v1.config import NodeConfig
from dbt_common.contracts.config.base import MergeBehavior
from dbt_common.contracts.constraints import ModelLevelConstraint
from dbt.artifacts.resources.v1.config import NodeConfig
from dbt.artifacts.resources.types import AccessType, NodeType
from dbt.artifacts.resources.v1.components import DeferRelation, NodeVersion, CompiledResource


@dataclass
Expand Down
10 changes: 7 additions & 3 deletions core/dbt/artifacts/resources/v1/saved_query.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
from __future__ import annotations
import time

import time
from dataclasses import dataclass, field
from typing import Any, Dict, List, Optional

from dbt_semantic_interfaces.type_enums.export_destination_type import (
ExportDestinationType,
)

from dbt.artifacts.resources.base import GraphResource
from dbt.artifacts.resources.v1.components import DependsOn, RefArgs
from dbt.artifacts.resources.v1.semantic_layer_components import (
Expand All @@ -10,8 +16,6 @@
)
from dbt_common.contracts.config.base import BaseConfig, CompareBehavior, MergeBehavior
from dbt_common.dataclass_schema import dbtClassMixin
from dbt_semantic_interfaces.type_enums.export_destination_type import ExportDestinationType
from typing import Any, Dict, List, Optional


@dataclass
Expand Down
11 changes: 8 additions & 3 deletions core/dbt/artifacts/resources/v1/seed.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
from dataclasses import dataclass, field
from typing import Optional, Literal
from dbt_common.dataclass_schema import ValidationError
from typing import Literal, Optional

from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.components import MacroDependsOn, DeferRelation, ParsedResource
from dbt.artifacts.resources.v1.components import (
DeferRelation,
MacroDependsOn,
ParsedResource,
)
from dbt.artifacts.resources.v1.config import NodeConfig
from dbt_common.dataclass_schema import ValidationError


@dataclass
Expand Down
10 changes: 7 additions & 3 deletions core/dbt/artifacts/resources/v1/semantic_layer_components.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
from dataclasses import dataclass
from dbt_common.dataclass_schema import dbtClassMixin
from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets
from dbt_semantic_interfaces.parsing.where_filter.where_filter_parser import WhereFilterParser
from typing import List, Sequence, Tuple

from dbt_semantic_interfaces.call_parameter_sets import FilterCallParameterSets
from dbt_semantic_interfaces.parsing.where_filter.where_filter_parser import (
WhereFilterParser,
)

from dbt_common.dataclass_schema import dbtClassMixin


@dataclass
class WhereFilter(dbtClassMixin):
Expand Down
Loading

0 comments on commit 29b8359

Please sign in to comment.