Skip to content

Commit

Permalink
Update pylint and associated config (#6517)
Browse files Browse the repository at this point in the history
* update pylint from 2.6.5 to 2.13.7
* removed outdated rule disables, added some more
* many small code style changes to match new linter rules
  • Loading branch information
smackesey committed Apr 21, 2022
1 parent 4181eba commit eab1d7d
Show file tree
Hide file tree
Showing 293 changed files with 777 additions and 703 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ def get_image_version(image_name):
"docker",
"images",
)
with open(os.path.join(root_images_path, image_name, "last_updated.yaml")) as f:
with open(
os.path.join(root_images_path, image_name, "last_updated.yaml"), encoding="utf8"
) as f:
versions = set(yaml.safe_load(f).values())

# There should be only one image timestamp tag across all Python versions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,23 +584,23 @@ def coverage_step():


def pylint_steps():
base_paths = [".buildkite", "bin", "docs/next/src"]
base_paths = [".buildkite", "scripts", "docs"]
base_paths_ext = ['"%s/**.py"' % p for p in base_paths]

return [
StepBuilder("pylint misc")
.run(
# Deps needed to pylint docs
"""pip install \
-e python_modules/dagster \
-e python_modules/dagster[test] \
-e python_modules/dagster-graphql \
-e python_modules/dagit \
-e python_modules/automation \
-e python_modules/libraries/dagstermill \
-e python_modules/libraries/dagster-celery \
-e python_modules/libraries/dagster-dask \
""",
"pylint -j 0 `git ls-files %s` --rcfile=.pylintrc" % " ".join(base_paths_ext),
"pylint -j 0 --rcfile=pyproject.toml `git ls-files %s`" % " ".join(base_paths_ext),
)
.on_integration_image(SupportedPython.V3_7)
.build()
Expand Down
2 changes: 1 addition & 1 deletion .buildkite/dagster-buildkite/dagster_buildkite/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def check_for_release():
return False

version = {}
with open("python_modules/dagster/dagster/version.py") as fp:
with open("python_modules/dagster/dagster/version.py", encoding="utf8") as fp:
exec(fp.read(), version) # pylint: disable=W0122

if git_tag == version["__version__"]:
Expand Down
32 changes: 0 additions & 32 deletions .pylintrc

This file was deleted.

12 changes: 9 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
# exit status. Prefix the command with "-" to instruct make to continue to the next command
# regardless of the preceding command's exit status.

pylint:
pylint -j 0 `git ls-files '*.py'` --rcfile=.pylintrc

# NOTE: See pyproject.toml [tool.black] for majority of black config. Only include/exclude options
# and format targets should be specified here. Note there are separate pyproject.toml for the root
# and examples/docs_snippets.
Expand Down Expand Up @@ -49,6 +46,15 @@ check_isort:
isort --check \
`git ls-files 'examples/docs_snippets/*.py'`

pylint:
pylint \
`git ls-files '.buildkite/*.py' 'examples/*.py' 'integration_tests/*.py' \
'helm/*.py' 'python_modules/*.py' 'scripts/*.py' \
':!:examples/airflow_ingest' \
':!:python_modules/libraries/dagster-airflow' \
':!:vendor' \
':!:snapshots'`

yamllint:
yamllint -c .yamllint.yaml --strict \
`git ls-files 'helm/*.yml' 'helm/*.yaml' ':!:helm/**/templates/*.yml' ':!:helm/**/templates/*.yaml'`
Expand Down
1 change: 1 addition & 0 deletions docs/content/concepts/ops-jobs-graphs/op-hooks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ if __name__ == "__main__":
with open(
file_relative_path(__file__, "prod_op_hooks.yaml"),
"r",
encoding="utf8",
) as fd:
run_config = yaml.safe_load(fd.read())
result = notif_all_dev.execute_in_process(
Expand Down
3 changes: 1 addition & 2 deletions docs/content/guides/dagster/dagster_type_factories.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ from dagster import DagsterType, check_dagster_type

set_containing_1 = DagsterType(
name="set_containing_1",
description=f"A set containing the value 1. May contain any other values.",
description="A set containing the value 1. May contain any other values.",
type_check_fn=lambda _context, obj: isinstance(obj, set) and 1 in obj,
)

Expand Down Expand Up @@ -115,7 +115,6 @@ generate_trip_distribution_plot.execute_in_process()
Since our logic looks OK, the problem likely originates in an incorrect assumption about the source data. To investigate, we write a [Pandera schema](https://pandera.readthedocs.io/en/stable/dataframe_schemas.html). The schema defines a set of dataframe columns with corresponding datatypes. Pandera columns are by default required to exist and non-nullable, so there is also an implicit expectation that columns with the specified names exist, and that all values are defined (NOTE: this illustrates only a small slice of Pandera's functionality):

```python file=/guides/dagster/dagster_type_factories/schema.py
import numpy as np
import pandas as pd
import pandera as pa

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def sort_by_calories(context, cereals):
)
os.makedirs(os.path.dirname(sorted_cereals_csv_path), exist_ok=True)

with open(sorted_cereals_csv_path, "w") as fd:
with open(sorted_cereals_csv_path, "w", encoding="utf8") as fd:
writer = csv.DictWriter(fd, fieldnames)
writer.writeheader()
writer.writerows(sorted_cereals)
Expand Down
4 changes: 2 additions & 2 deletions docs/pack_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@


def read_json(filename):
with open(filename) as f:
with open(filename, encoding="utf8") as f:
data = json.load(f)
return data


def write_json(filename, data):
with open(filename, "w") as f:
with open(filename, "w", encoding="utf8") as f:
json.dump(data, f, sort_keys=True)


Expand Down
4 changes: 2 additions & 2 deletions docs/screenshot_capture/capture-screenshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
from typing import Any, Mapping, Sequence

import yaml
from selenium import webdriver
from selenium import webdriver # pylint: disable=import-error


def load_screenshot_specs(path) -> Sequence[Mapping]:
with open(path, "r") as f:
with open(path, "r", encoding="utf8") as f:
return yaml.safe_load(f)


Expand Down
2 changes: 1 addition & 1 deletion docs/screenshot_capture/match_screenshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@


def match_screenshots():
with open("./docs/screenshot_capture/screenshots.yaml", "r") as f:
with open("./docs/screenshot_capture/screenshots.yaml", "r", encoding="utf8") as f:
screenshot_specs = yaml.load(f)

for screenshot_spec in screenshot_specs:
Expand Down
4 changes: 2 additions & 2 deletions docs/sphinx/_ext/autodoc_configurable.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from dagster.config.config_type import ConfigType, ConfigTypeKind
from dagster.core.definitions.configurable import ConfigurableDefinition
from dagster.serdes import ConfigurableClass
from sphinx.ext.autodoc import DataDocumenter
from sphinx.ext.autodoc import DataDocumenter # pylint: disable=import-error,no-name-in-module


def type_repr(config_type: ConfigType) -> str:
Expand Down Expand Up @@ -97,7 +97,7 @@ class ConfigurableDocumenter(DataDocumenter):
directivetype = "data"

@classmethod
def can_document_member(cls, member: Any, membername: str, isattr: bool, parent: Any) -> bool:
def can_document_member(cls, member: Any, _membername: str, _isattr: bool, _parent: Any) -> bool:
return isinstance(member, ConfigurableDefinition) or isinstance(member, ConfigurableClass)

def add_content(self, more_content, no_docstring: bool = False) -> None:
Expand Down
2 changes: 1 addition & 1 deletion examples/airflow_ingest/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ commands =
[testenv:pylint]
basepython = python
commands =
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../.pylintrc airflow_ingest/'
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../pyproject.toml airflow_ingest/'
2 changes: 1 addition & 1 deletion examples/airline_demo/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ whitelist_externals =
pylint
basepython = python
commands =
pylint -j 0 --rcfile=../../.pylintrc airline_demo airline_demo_tests
pylint -j 0 --rcfile=../../pyproject.toml airline_demo airline_demo_tests
2 changes: 1 addition & 1 deletion examples/basic_pyspark/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ commands =
[testenv:pylint]
basepython = python
commands =
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../.pylintrc basic_pyspark/'
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../pyproject.toml basic_pyspark/'
4 changes: 2 additions & 2 deletions examples/bollinger/bollinger/resources/csv_io_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def handle_output(self, context, obj: pd.DataFrame):
fpath = self._get_fs_path(context.asset_key)
os.makedirs(os.path.dirname(fpath), exist_ok=True)
obj.to_csv(fpath)
with open(fpath + ".version", "w") as f:
with open(fpath + ".version", "w", encoding="utf8") as f:
f.write(context.version if context.version else "None")

yield MetadataEntry.int(obj.shape[0], "Rows")
Expand Down Expand Up @@ -66,7 +66,7 @@ def has_output(self, context) -> bool:
version_fpath = fpath + ".version"
if not os.path.exists(version_fpath):
return False
with open(version_fpath, "r") as f:
with open(version_fpath, "r", encoding="utf8") as f:
version = f.read()

return version == context.version
Expand Down
1 change: 1 addition & 0 deletions examples/bollinger/scripts/download_stock_data.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env python
# pylint: disable=print-call

import os
import sys
Expand Down
2 changes: 1 addition & 1 deletion examples/bollinger/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ commands =
whitelist_externals =
pylint
commands =
pylint -j 0 --rcfile=../../.pylintrc bollinger bollinger_tests
pylint -j 0 --rcfile=../../pyproject.toml bollinger bollinger_tests

[testenv:mypy]
whitelist_externals =
Expand Down
2 changes: 1 addition & 1 deletion examples/dbt_example/dbt_example/solids.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def download_file(context) -> str:
url = context.solid_config["url"]
target_path = context.solid_config["target_path"]

with open(target_path, "w") as fd:
with open(target_path, "w", encoding="utf8") as fd:
fd.write(requests.get(url).text)

return target_path
Expand Down
2 changes: 1 addition & 1 deletion examples/dbt_example/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ commands =
[testenv:pylint]
basepython = python
commands =
pylint -j 0 --rcfile=../../.pylintrc dbt_example/
pylint -j 0 --rcfile=../../pyproject.toml dbt_example/
2 changes: 1 addition & 1 deletion examples/deploy_docker/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ commands =
[testenv:pylint]
basepython = python
commands =
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../.pylintrc deploy_docker/'
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../pyproject.toml deploy_docker/'
10 changes: 5 additions & 5 deletions examples/deploy_ecs/tests/test_deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def source_code(reference_deployment, tmpdir):
shutil.copytree(source, modules / requirement, ignore=shutil.ignore_patterns(".tox"))
overrides.append(f"./modules/{requirement}\n")

with open(path, "w") as f:
with open(path, "w", encoding="utf8") as f:
f.writelines(overrides)

return modules
Expand All @@ -82,13 +82,13 @@ def source_code(reference_deployment, tmpdir):
@pytest.fixture
def overridden_dockerfile(source_code):
# Override Dockerfile to copy our source code into the container
with open("Dockerfile", "r") as f:
with open("Dockerfile", "r", encoding="utf8") as f:
dockerfile = f.readlines()
# Copy the files in directly after we set the WORKDIR
index = dockerfile.index("WORKDIR $DAGSTER_HOME\n") + 1
copy = ["RUN mkdir -p $DAGSTER_HOME/modules\n", "COPY modules $DAGSTER_HOME/modules\n"]
dockerfile = dockerfile[:index] + copy + dockerfile[index:]
with open("Dockerfile", "w") as f:
with open("Dockerfile", "w", encoding="utf8") as f:
f.writelines(dockerfile)


Expand All @@ -98,13 +98,13 @@ def overridden_dagster_yaml(reference_deployment):
# run on a real ECS cluster whereas DefaultRunLauncher can successfully run
# end-to-end on a local ECS simulation. This is because the local ECS
# simulation doesn't mock out the ECS API in its entirety.
with open("dagster.yaml", "r") as f:
with open("dagster.yaml", "r", encoding="utf8") as f:
dagster_yaml = yaml.safe_load(f)
dagster_yaml["run_launcher"] = {
"module": "dagster.core.launcher",
"class": "DefaultRunLauncher",
}
with open("dagster.yaml", "w") as f:
with open("dagster.yaml", "w", encoding="utf8") as f:
f.write(yaml.safe_dump(dagster_yaml))


Expand Down
2 changes: 1 addition & 1 deletion examples/deploy_ecs/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ commands =
[testenv:pylint]
basepython = python
commands =
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../.pylintrc deploy_ecs/'
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../pyproject.toml deploy_ecs/'
2 changes: 1 addition & 1 deletion examples/deploy_k8s/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ commands =
[testenv:pylint]
basepython = python
commands =
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../.pylintrc deploy_k8s/example_project'
/bin/bash -c 'cd .. && pylint -j 0 --rcfile=../pyproject.toml deploy_k8s/example_project'
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# isort: skip_file
# pylint: disable=unnecessary-ellipsis

from dagster import repository, DefaultSensorStatus, SkipReason

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# isort: skip_file
# pylint: disable=unused-argument
# pylint: disable=reimported
# pylint: disable=unused-argument,reimported,unnecessary-ellipsis
from dagster import ResourceDefinition, graph, job


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def repo():
with open(
file_relative_path(__file__, "prod_op_hooks.yaml"),
"r",
encoding="utf8",
) as fd:
run_config = yaml.safe_load(fd.read())
result = notif_all_dev.execute_in_process(
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=print-call
from dagster import job, op


Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# isort: skip_file
# pylint: disable=print-call

# start_pipeline_marker

Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import numpy as np
import pandas as pd
import pandera as pa

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
# pylint: disable=expression-not-assigned
# one_off_type_start

from dagster import DagsterType, check_dagster_type

set_containing_1 = DagsterType(
name="set_containing_1",
description=f"A set containing the value 1. May contain any other values.",
description="A set containing the value 1. May contain any other values.",
type_check_fn=lambda _context, obj: isinstance(obj, set) and 1 in obj,
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# isort: skip_file
# pylint: disable=unused-variable
CUSTOM_HEADER_NAME = "X-SOME-HEADER"
# start_custom_run_coordinator_marker

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def sort_by_calories(context, cereals):
)
os.makedirs(os.path.dirname(sorted_cereals_csv_path), exist_ok=True)

with open(sorted_cereals_csv_path, "w") as fd:
with open(sorted_cereals_csv_path, "w", encoding="utf8") as fd:
writer = csv.DictWriter(fd, fieldnames)
writer.writeheader()
writer.writerows(sorted_cereals)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
@op
def load_cereals():
dataset_path = os.path.join(os.path.dirname(__file__), "cereal.csv")
with open(dataset_path, "r") as fd:
with open(dataset_path, "r", encoding="utf8") as fd:
cereals = [row for row in csv.DictReader(fd)]
return cereals

Expand Down

1 comment on commit eab1d7d

@vercel
Copy link

@vercel vercel bot commented on eab1d7d Apr 21, 2022

Choose a reason for hiding this comment

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

Please sign in to comment.