Skip to content

Commit

Permalink
Enable line-too-long for pylint (#5085)
Browse files Browse the repository at this point in the history
* Enable line-too-long for pylint

Signed-off-by: harupy <hkawamura0130@gmail.com>

* replace noqa

Signed-off-by: harupy <hkawamura0130@gmail.com>

* lint

Signed-off-by: harupy <hkawamura0130@gmail.com>

* fix

Signed-off-by: harupy <hkawamura0130@gmail.com>

* define multi_context

Signed-off-by: harupy <hkawamura0130@gmail.com>

* use mock_method_chain

Signed-off-by: harupy <hkawamura0130@gmail.com>

* shorten URIs

Signed-off-by: harupy <hkawamura0130@gmail.com>

* fix test_gcs_artifact_repo.py

Signed-off-by: harupy <hkawamura0130@gmail.com>

* remove noaq

Signed-off-by: harupy <hkawamura0130@gmail.com>

* use list

Signed-off-by: harupy <hkawamura0130@gmail.com>

* fix examples

Signed-off-by: harupy <hkawamura0130@gmail.com>

* always set return_value and side_effect

Signed-off-by: harupy <hkawamura0130@gmail.com>

* enforce keyword arguments

Signed-off-by: harupy <hkawamura0130@gmail.com>
  • Loading branch information
harupy committed Nov 22, 2021
1 parent 9faf3b0 commit 22e69c3
Show file tree
Hide file tree
Showing 18 changed files with 203 additions and 78 deletions.
2 changes: 1 addition & 1 deletion dev/set_matrix.py
Expand Up @@ -562,7 +562,7 @@ def main(args):

if "GITHUB_ACTIONS" in os.environ:
# `::set-output` is a special syntax for GitHub Actions to set an action's output parameter.
# https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-output-parameter # noqa
# https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-output-parameter
# Note that this actually doesn't print anything to the console.
print("::set-output name=matrix::{}".format(json.dumps(matrix)))

Expand Down
2 changes: 1 addition & 1 deletion mlflow/pytorch/_pytorch_autolog.py
Expand Up @@ -69,7 +69,7 @@ def _log_metrics(self, trainer, pl_module):
# pytorch-lightning runs a few steps of validation in the beginning of training
# as a sanity check to catch bugs without having to wait for the training routine
# to complete. During this check, we should skip logging metrics.
# https://pytorch-lightning.readthedocs.io/en/stable/common/trainer.html#num-sanity-val-steps # noqa
# https://pytorch-lightning.readthedocs.io/en/stable/common/trainer.html#num-sanity-val-steps
sanity_checking = (
# `running_sanity_check` has been renamed to `sanity_checking`:
# https://github.com/PyTorchLightning/pytorch-lightning/pull/9209
Expand Down
4 changes: 2 additions & 2 deletions mlflow/sklearn/utils.py
Expand Up @@ -92,7 +92,7 @@ def _get_sample_weight(arg_names, args, kwargs):

# In most cases, X_var_name and y_var_name become "X" and "y", respectively.
# However, certain sklearn models use different variable names for X and y.
# E.g., see: https://scikit-learn.org/stable/modules/generated/sklearn.multioutput.MultiOutputClassifier.html#sklearn.multioutput.MultiOutputClassifier.fit # noqa: E501
# E.g., see: https://scikit-learn.org/stable/modules/generated/sklearn.multioutput.MultiOutputClassifier.html#sklearn.multioutput.MultiOutputClassifier.fit
X_var_name, y_var_name = fit_arg_names[:2]
Xy = _get_Xy(fit_args, fit_kwargs, X_var_name, y_var_name)
sample_weight = (
Expand Down Expand Up @@ -598,7 +598,7 @@ def _create_child_runs_for_parameter_search(
parameter search estimator - `cv_estimator`, which provides relevant performance
metrics for each point in the parameter search space. One child run is created
for each point in the parameter search space. For additional information, see
`https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.GridSearchCV.html`_. # noqa: E501
`https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.GridSearchCV.html`_.
:param autologging_client: An instance of `MlflowAutologgingQueueingClient` used for
efficiently logging run data to MLflow Tracking.
Expand Down
4 changes: 2 additions & 2 deletions mlflow/tracking/client.py
Expand Up @@ -1235,7 +1235,7 @@ def _is_numpy_array(image):
return isinstance(image, np.ndarray)

def _normalize_to_uint8(x):
# Based on: https://github.com/matplotlib/matplotlib/blob/06567e021f21be046b6d6dcf00380c1cb9adaf3c/lib/matplotlib/image.py#L684 # noqa
# Based on: https://github.com/matplotlib/matplotlib/blob/06567e021f21be046b6d6dcf00380c1cb9adaf3c/lib/matplotlib/image.py#L684

is_int = np.issubdtype(x.dtype, np.integer)
low = 0
Expand Down Expand Up @@ -1268,7 +1268,7 @@ def _normalize_to_uint8(x):
"Please install it via: pip install Pillow"
) from exc

# Ref.: https://numpy.org/doc/stable/reference/generated/numpy.dtype.kind.html#numpy-dtype-kind # noqa
# Ref.: https://numpy.org/doc/stable/reference/generated/numpy.dtype.kind.html#numpy-dtype-kind
valid_data_types = {
"b": "bool",
"i": "signed integer",
Expand Down
2 changes: 1 addition & 1 deletion mlflow/xgboost/__init__.py
Expand Up @@ -426,7 +426,7 @@ def record_eval_results(eval_results, metrics_logger):

# In xgboost >= 1.3.0, user-defined callbacks should inherit
# `xgboost.callback.TrainingCallback`:
# https://xgboost.readthedocs.io/en/latest/python/callbacks.html#defining-your-own-callback # noqa
# https://xgboost.readthedocs.io/en/latest/python/callbacks.html#defining-your-own-callback
return AutologCallback(metrics_logger, eval_results)
else:
from mlflow.xgboost._autolog import autolog_callback
Expand Down
5 changes: 3 additions & 2 deletions pylintrc
Expand Up @@ -177,7 +177,8 @@ disable=print-statement,
# either give multiple identifier separated by comma (,) or put this option
# multiple time (only on the command line, not in the configuration file where
# it should appear only once). See also the "--disable" option for examples.
enable=c-extension-no-member
enable=c-extension-no-member,
line-too-long


[REPORTS]
Expand Down Expand Up @@ -330,7 +331,7 @@ variable-naming-style=snake_case
expected-line-ending-format=

# Regexp for a line that is allowed to be longer than the limit.
ignore-long-lines=^\s*(# )?<?https?://\S+>?$
ignore-long-lines=https?://\S+

# Number of spaces of indent required inside a hanging or continued line.
indent-after-paren=4
Expand Down
4 changes: 2 additions & 2 deletions tests/autologging/test_autologging_safety_unit.py
Expand Up @@ -644,7 +644,7 @@ def patch_impl(original, *args, **kwargs):
assert patch_success.exception is original_success.exception is None


def test_safe_patch_makes_expected_event_logging_calls_when_patch_implementation_throws_and_original_succeeds( # noqa
def test_safe_patch_makes_expected_event_logging_calls_when_patch_implementation_throws_and_original_succeeds( # pylint: disable=line-too-long
patch_destination,
test_autologging_integration,
mock_event_logger,
Expand Down Expand Up @@ -684,7 +684,7 @@ def patch_impl(original, *args, **kwargs):
assert patch_error.exception == exc_to_raise


def test_safe_patch_makes_expected_event_logging_calls_when_patch_implementation_throws_and_original_throws( # noqa
def test_safe_patch_makes_expected_event_logging_calls_when_patch_implementation_throws_and_original_throws( # pylint: disable=line-too-long
patch_destination,
test_autologging_integration,
mock_event_logger,
Expand Down
41 changes: 40 additions & 1 deletion tests/helper_functions.py
@@ -1,6 +1,9 @@
import os
import random
import functools
from unittest import mock
from contextlib import ExitStack, contextmanager


import requests
import time
Expand Down Expand Up @@ -241,7 +244,7 @@ def __exit__(self, tp, val, traceback):
pgrp = os.getpgid(self._proc.pid)
os.killpg(pgrp, signal.SIGTERM)
else:
# https://stackoverflow.com/questions/47016723/windows-equivalent-for-spawning-and-killing-separate-process-group-in-python-3 # noqa
# https://stackoverflow.com/questions/47016723/windows-equivalent-for-spawning-and-killing-separate-process-group-in-python-3
self._proc.send_signal(signal.CTRL_BREAK_EVENT)
self._proc.kill()

Expand Down Expand Up @@ -406,3 +409,39 @@ def decorator(f):
return pytest.mark.allow_infer_pip_requirements_fallback(f) if condition else f

return decorator


def mock_method_chain(mock_obj, methods, return_value=None, side_effect=None):
"""
Mock a chain of methods.
Examples
--------
>>> from unittest import mock
>>> m = mock.MagicMock()
>>> mock_method_chain(m, ["a", "b"], return_value=0)
>>> m.a().b()
0
>>> mock_method_chain(m, ["c.d", "e"], return_value=1)
>>> m.c.d().e()
1
>>> mock_method_chain(m, ["f"], side_effect=Exception("side_effect"))
>>> m.f()
Traceback (most recent call last):
...
Exception: side_effect
"""
length = len(methods)
for idx, method in enumerate(methods):
mock_obj = functools.reduce(getattr, method.split("."), mock_obj)
if idx != length - 1:
mock_obj = mock_obj.return_value
else:
mock_obj.return_value = return_value
mock_obj.side_effect = side_effect


@contextmanager
def multi_context(*cms):
with ExitStack() as stack:
yield list(map(stack.enter_context, cms))
10 changes: 5 additions & 5 deletions tests/resources/mlflow-test-plugin/setup.py
Expand Up @@ -13,17 +13,17 @@
# Define a Tracking Store plugin for tracking URIs with scheme 'file-plugin'
"mlflow.tracking_store": "file-plugin=mlflow_test_plugin.file_store:PluginFileStore",
# Define a ArtifactRepository plugin for artifact URIs with scheme 'file-plugin'
"mlflow.artifact_repository": "file-plugin=mlflow_test_plugin.local_artifact:PluginLocalArtifactRepository", # noqa
"mlflow.artifact_repository": "file-plugin=mlflow_test_plugin.local_artifact:PluginLocalArtifactRepository", # pylint: disable=line-too-long
# Define a RunContextProvider plugin. The entry point name for run context providers
# is not used, and so is set to the string "unused" here
"mlflow.run_context_provider": "unused=mlflow_test_plugin.run_context_provider:PluginRunContextProvider", # noqa
"mlflow.run_context_provider": "unused=mlflow_test_plugin.run_context_provider:PluginRunContextProvider", # pylint: disable=line-too-long
# Define a RequestHeaderProvider plugin. The entry point name for request header providers
# is not used, and so is set to the string "unused" here
"mlflow.request_header_provider": "unused=mlflow_test_plugin.request_header_provider:PluginRequestHeaderProvider", # noqa
"mlflow.request_header_provider": "unused=mlflow_test_plugin.request_header_provider:PluginRequestHeaderProvider", # pylint: disable=line-too-long
# Define a Model Registry Store plugin for tracking URIs with scheme 'file-plugin'
"mlflow.model_registry_store": "file-plugin=mlflow_test_plugin.sqlalchemy_store:PluginRegistrySqlAlchemyStore", # noqa
"mlflow.model_registry_store": "file-plugin=mlflow_test_plugin.sqlalchemy_store:PluginRegistrySqlAlchemyStore", # pylint: disable=line-too-long
# Define a MLflow Project Backend plugin called 'dummy-backend'
"mlflow.project_backend": "dummy-backend=mlflow_test_plugin.dummy_backend:PluginDummyProjectBackend", # noqa
"mlflow.project_backend": "dummy-backend=mlflow_test_plugin.dummy_backend:PluginDummyProjectBackend", # pylint: disable=line-too-long
# Define a MLflow model deployment plugin for target 'faketarget'
"mlflow.deployments": "faketarget=mlflow_test_plugin.fake_deployment_plugin",
},
Expand Down
3 changes: 2 additions & 1 deletion tests/shap/test_log.py
Expand Up @@ -293,7 +293,8 @@ def test_pyfunc_serve_and_score():
# `link` defaults to `shap.links.identity` which is decorated by `numba.jit` and causes
# the following error when loading the explainer for serving:
# ```
# Exception: The passed link function needs to be callable and have a callable .inverse property! # noqa
# Exception: The passed link function needs to be callable and have a callable
# .inverse property!
# ```
# As a workaround, use an identify function that's NOT decorated by `numba.jit`.
link=create_identity_function(),
Expand Down
2 changes: 1 addition & 1 deletion tests/store/artifact/test_databricks_artifact_repo.py
Expand Up @@ -152,7 +152,7 @@ def test_init_artifact_uri(self, artifact_uri, expected_uri, expected_db_uri):
("dbfs:/databricks/mlflow-tracking/MOCK-EXP/MOCK-RUN-ID/artifacts", ""),
("dbfs:/databricks/mlflow-tracking/MOCK-EXP/MOCK-RUN-ID/artifacts/arty", "arty"),
(
"dbfs://prof@databricks/databricks/mlflow-tracking/MOCK-EXP/MOCK-RUN-ID/artifacts/arty", # noqa
"dbfs://prof@databricks/databricks/mlflow-tracking/MOCK-EXP/MOCK-RUN-ID/artifacts/arty", # pylint: disable=line-too-long
"arty",
),
(
Expand Down
58 changes: 47 additions & 11 deletions tests/store/artifact/test_gcs_artifact_repo.py
Expand Up @@ -5,10 +5,11 @@
from unittest import mock

from google.cloud.storage import client as gcs_client
from google.auth.exceptions import DefaultCredentialsError

from mlflow.store.artifact.artifact_repository_registry import get_artifact_repository
from mlflow.store.artifact.gcs_artifact_repo import GCSArtifactRepository
from google.auth.exceptions import DefaultCredentialsError
from tests.helper_functions import mock_method_chain


@pytest.fixture
Expand Down Expand Up @@ -123,8 +124,15 @@ def test_log_artifact(gcs_mock, tmpdir):

# This will call isfile on the code path being used,
# thus testing that it's being called with an actually file path
gcs_mock.Client.return_value.bucket.return_value.blob.return_value.upload_from_filename.side_effect = ( # noqa
os.path.isfile
mock_method_chain(
gcs_mock,
[
"Client",
"bucket",
"blob",
"upload_from_filename",
],
side_effect=os.path.isfile,
)
repo.log_artifact(fpath)

Expand All @@ -141,8 +149,15 @@ def test_log_artifacts(gcs_mock, tmpdir):
subd.join("b.txt").write("B")
subd.join("c.txt").write("C")

gcs_mock.Client.return_value.bucket.return_value.blob.return_value.upload_from_filename.side_effect = ( # noqa
os.path.isfile
mock_method_chain(
gcs_mock,
[
"Client",
"bucket",
"blob",
"upload_from_filename",
],
side_effect=os.path.isfile,
)
repo.log_artifacts(subd.strpath)

Expand All @@ -165,8 +180,15 @@ def mkfile(fname):
f = tmpdir.join(fname)
f.write("hello world!")

gcs_mock.Client.return_value.bucket.return_value.blob.return_value.download_to_filename.side_effect = ( # noqa
mkfile
mock_method_chain(
gcs_mock,
[
"Client",
"bucket",
"blob",
"download_to_filename",
],
side_effect=mkfile,
)

repo.download_artifacts("test.txt")
Expand Down Expand Up @@ -230,10 +252,24 @@ def mkfile(fname):
f = tmpdir.join(fname)
f.write("hello world!")

gcs_mock.Client.return_value.bucket.return_value.list_blobs.side_effect = get_mock_listing

gcs_mock.Client.return_value.bucket.return_value.blob.return_value.download_to_filename.side_effect = ( # noqa
mkfile
mock_method_chain(
gcs_mock,
[
"Client",
"bucket",
"list_blobs",
],
side_effect=get_mock_listing,
)
mock_method_chain(
gcs_mock,
[
"Client",
"bucket",
"blob",
"download_to_filename",
],
side_effect=mkfile,
)

# Ensure that the root directory can be downloaded successfully
Expand Down
8 changes: 7 additions & 1 deletion tests/tracking/context/test_databricks_job_context.py
Expand Up @@ -10,6 +10,7 @@
MLFLOW_DATABRICKS_WEBAPP_URL,
)
from mlflow.tracking.context.databricks_job_context import DatabricksJobRunContext
from tests.helper_functions import multi_context


def test_databricks_job_run_context_in_context():
Expand All @@ -23,7 +24,12 @@ def test_databricks_job_run_context_tags():
patch_job_type = mock.patch("mlflow.utils.databricks_utils.get_job_type")
patch_webapp_url = mock.patch("mlflow.utils.databricks_utils.get_webapp_url")

with patch_job_id as job_id_mock, patch_job_run_id as job_run_id_mock, patch_job_type as job_type_mock, patch_webapp_url as webapp_url_mock: # noqa
with multi_context(patch_job_id, patch_job_run_id, patch_job_type, patch_webapp_url) as (
job_id_mock,
job_run_id_mock,
job_type_mock,
webapp_url_mock,
):
assert DatabricksJobRunContext().tags() == {
MLFLOW_SOURCE_NAME: "jobs/{job_id}/run/{job_run_id}".format(
job_id=job_id_mock.return_value, job_run_id=job_run_id_mock.return_value
Expand Down
7 changes: 6 additions & 1 deletion tests/tracking/context/test_databricks_notebook_context.py
Expand Up @@ -9,6 +9,7 @@
MLFLOW_DATABRICKS_WEBAPP_URL,
)
from mlflow.tracking.context.databricks_notebook_context import DatabricksNotebookRunContext
from tests.helper_functions import multi_context


def test_databricks_notebook_run_context_in_context():
Expand All @@ -21,7 +22,11 @@ def test_databricks_notebook_run_context_tags():
patch_notebook_path = mock.patch("mlflow.utils.databricks_utils.get_notebook_path")
patch_webapp_url = mock.patch("mlflow.utils.databricks_utils.get_webapp_url")

with patch_notebook_id as notebook_id_mock, patch_notebook_path as notebook_path_mock, patch_webapp_url as webapp_url_mock: # noqa
with multi_context(patch_notebook_id, patch_notebook_path, patch_webapp_url) as (
notebook_id_mock,
notebook_path_mock,
webapp_url_mock,
):
assert DatabricksNotebookRunContext().tags() == {
MLFLOW_SOURCE_NAME: notebook_path_mock.return_value,
MLFLOW_SOURCE_TYPE: SourceType.to_string(SourceType.NOTEBOOK),
Expand Down

0 comments on commit 22e69c3

Please sign in to comment.