From 3259a43d2ca3cdc679262a398f09219b522efa50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?= =?UTF-8?q?=E0=A4=A4=29?= Date: Mon, 29 Jun 2020 11:37:20 +0545 Subject: [PATCH] pylint: enable additional checks Created a separate pylint plugin to disable some checks for tests. --- .pylintrc | 20 ++++---- dvc/api.py | 2 +- dvc/cache.py | 1 + dvc/cli.py | 2 +- dvc/command/git_hook.py | 3 ++ dvc/dvcfile.py | 8 ++-- dvc/external_repo.py | 1 + dvc/lock.py | 2 +- dvc/logger.py | 1 + dvc/main.py | 9 ++-- dvc/output/base.py | 5 ++ dvc/path_info.py | 7 +-- dvc/remote/base.py | 15 ++++-- dvc/remote/gs.py | 4 +- dvc/remote/local.py | 7 ++- dvc/remote/ssh/connection.py | 2 +- dvc/repo/__init__.py | 1 + dvc/repo/add.py | 2 +- dvc/repo/gc.py | 7 ++- dvc/repo/plots/data.py | 2 +- dvc/repo/plots/template.py | 4 ++ dvc/repo/pull.py | 4 +- dvc/repo/tree.py | 10 ++-- dvc/scm/git/__init__.py | 4 +- dvc/scm/git/tree.py | 2 +- dvc/stage/decorators.py | 4 +- dvc/stage/loader.py | 2 +- dvc/stage/run.py | 5 +- dvc/stage/serialize.py | 5 +- dvc/state.py | 2 +- dvc/system.py | 2 +- dvc/utils/fs.py | 2 +- fastentrypoints.py | 2 +- setup.cfg | 2 +- setup.py | 1 + tests/func/test_api.py | 4 +- tests/func/test_checkout.py | 3 +- tests/func/test_data_cloud.py | 2 +- tests/func/test_dvcfile.py | 1 + tests/func/test_repro.py | 22 ++++----- tests/func/test_repro_multistage.py | 2 +- tests/func/test_state.py | 2 +- tests/func/test_tree.py | 1 + tests/pylint_plugin_disable.py | 47 +++++++++++++++++++ tests/remotes/http.py | 2 +- tests/remotes/ssh.py | 2 +- tests/unit/output/test_local.py | 2 +- tests/unit/remote/test_base.py | 2 +- tests/unit/remote/test_slow_link_detection.py | 4 +- tests/unit/stage/test_stage.py | 5 +- 50 files changed, 175 insertions(+), 78 deletions(-) create mode 100644 tests/pylint_plugin_disable.py diff --git a/.pylintrc b/.pylintrc index 08f96e765a..e036b6217b 100644 --- a/.pylintrc +++ b/.pylintrc @@ -15,7 +15,11 @@ ignore-patterns= # Python code to execute, usually for sys.path manipulation such as # pygtk.require(). -#init-hook= +init-hook="import sys; \ + from pathlib import Path; \ + from pylint.config import find_pylintrc; \ + sys.path.append(Path(find_pylintrc()).parent / 'tests');" + # Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the # number of processors available to use. @@ -28,7 +32,7 @@ limit-inference-results=100 # List of plugins (as comma separated values of python modules names) to load, # usually to register additional checkers. -load-plugins=pylint_pytest +load-plugins=pylint_pytest,pylint_plugin_disable # Pickle collected data for later comparisons. persistent=yes @@ -149,10 +153,8 @@ disable=format, exception-escape, comprehension-escape, # custom disables start here - protected-access, # needs-refactor no-self-use, invalid-name, - blacklisted-name, # already a check enabled for `wildcard-import` unused-wildcard-import, misplaced-comparison-constant, @@ -176,10 +178,6 @@ disable=format, # we use it to speedup/optimize import-outside-toplevel, fixme, - # Enabling soon: - no-member, - unused-argument, - arguments-differ, # Enable the message, report, category or checker with the given id(s). You can @@ -267,7 +265,7 @@ contextmanager-decorators=contextlib.contextmanager # List of members which are set dynamically and missed by pylint inference # system, and so shouldn't trigger E1101 when accessed. Python regular # expressions are accepted. -generated-members= +generated-members=pytest.lazy_fixture,logging.TRACE,logger.trace,sys.getwindowsversion # Tells whether missing members accessed in mixin class should be ignored. A # mixin class is detected if its name ends with "mixin" (case insensitive). @@ -288,7 +286,7 @@ ignore-on-opaque-inference=yes # List of class names for which member attributes should not be checked (useful # for classes with dynamically set attributes). This supports the use of # qualified names. -ignored-classes=optparse.Values,thread._local,_thread._local +ignored-classes=optparse.Values,thread._local,_thread._local,Dvcfile # List of module names for which member attributes should not be checked # (useful for modules/projects where namespaces are manipulated during runtime @@ -329,7 +327,7 @@ dummy-variables-rgx=_+$|(_[a-zA-Z0-9_]*[a-zA-Z0-9]+?$)|dummy|^ignored_|^unused_ # Argument names that match this expression will be ignored. Default to name # with leading underscore. -ignored-argument-names=_.*|^ignored_|^unused_ +ignored-argument-names=_.*|^ignored_|^unused_|args|kwargs # Tells whether we should check for unused import in __init__ files. init-import=no diff --git a/dvc/api.py b/dvc/api.py index f51e8287d3..e469d24063 100644 --- a/dvc/api.py +++ b/dvc/api.py @@ -27,7 +27,7 @@ def get_url(path, repo=None, rev=None, remote=None): """ with _make_repo(repo, rev=rev) as _repo: if not isinstance(_repo, Repo): - raise UrlNotDvcRepoError(_repo.url) + raise UrlNotDvcRepoError(_repo.url) # pylint: disable=no-member out = _repo.find_out_by_relpath(path) remote_obj = _repo.cloud.get_remote(remote) return str(remote_obj.hash_to_path_info(out.checksum)) diff --git a/dvc/cache.py b/dvc/cache.py index acc2719042..291808b9a1 100644 --- a/dvc/cache.py +++ b/dvc/cache.py @@ -106,6 +106,7 @@ def update(self, item, suffix=""): class NamedCache: + # pylint: disable=protected-access def __init__(self): self._items = defaultdict(lambda: defaultdict(NamedCacheItem)) self.external = defaultdict(set) diff --git a/dvc/cli.py b/dvc/cli.py index 9f3e39807a..70f8e0fdb1 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -83,7 +83,7 @@ class DvcParser(argparse.ArgumentParser): """Custom parser class for dvc CLI.""" - def error(self, message, command=None): + def error(self, message, command=None): # pylint: disable=arguments-differ """Custom error method. Args: message (str): error message. diff --git a/dvc/command/git_hook.py b/dvc/command/git_hook.py index 11c8b59ca8..e82cc6f624 100644 --- a/dvc/command/git_hook.py +++ b/dvc/command/git_hook.py @@ -19,6 +19,9 @@ def run(self): return self._run() + def _run(self): + raise NotImplementedError + class CmdPreCommit(CmdHookBase): def _run(self): diff --git a/dvc/dvcfile.py b/dvc/dvcfile.py index 3aeefb3992..8269edaa8a 100644 --- a/dvc/dvcfile.py +++ b/dvc/dvcfile.py @@ -108,7 +108,7 @@ def validate(cls, d, fname=None): except MultipleInvalid as exc: raise StageFileFormatError(f"'{fname}' format error: {exc}") - def remove(self, force=False): + def remove(self, force=False): # pylint: disable=unused-argument with contextlib.suppress(FileNotFoundError): os.unlink(self.path) @@ -141,7 +141,7 @@ def dump(self, stage, **kwargs): dump_yaml(self.path, serialize.to_single_stage_file(stage)) self.repo.scm.track_file(self.relpath) - def remove_stage(self, stage): + def remove_stage(self, stage): # pylint: disable=unused-argument self.remove() @@ -154,7 +154,9 @@ class PipelineFile(FileMixin): def _lockfile(self): return Lockfile(self.repo, os.path.splitext(self.path)[0] + ".lock") - def dump(self, stage, update_pipeline=False, no_lock=False): + def dump( + self, stage, update_pipeline=False, no_lock=False, **kwargs + ): # pylint: disable=arguments-differ """Dumps given stage appropriately in the dvcfile.""" from dvc.stage import PipelineStage diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 2efa0414c9..5f22321473 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -68,6 +68,7 @@ def clean_repos(): class BaseExternalRepo: + # pylint: disable=no-member _local_cache = None diff --git a/dvc/lock.py b/dvc/lock.py index 78cf35bbde..cfe7fd0539 100644 --- a/dvc/lock.py +++ b/dvc/lock.py @@ -119,7 +119,7 @@ def __init__( def lockfile(self): return self._lockfile - def lock(self): + def lock(self): # pylint: disable=arguments-differ try: super().lock(timedelta(seconds=DEFAULT_TIMEOUT)) except flufl.lock.TimeOutError: diff --git a/dvc/logger.py b/dvc/logger.py index 25671e3d64..d48dc3437e 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -34,6 +34,7 @@ def addLoggingLevel(levelName, levelNum, methodName=None): def logForLevel(self, message, *args, **kwargs): if self.isEnabledFor(levelNum): + # pylint: disable=protected-access self._log(levelNum, message, args, **kwargs) def logToRoot(message, *args, **kwargs): diff --git a/dvc/main.py b/dvc/main.py index 1c2ac8671a..7bf18bb2cc 100644 --- a/dvc/main.py +++ b/dvc/main.py @@ -65,8 +65,8 @@ def main(argv=None): except DvcException: ret = 255 logger.exception("") - except Exception as exc: # pylint: disable=broad-except - if isinstance(exc, OSError) and exc.errno == errno.EMFILE: + except OSError as exc: + if exc.errno == errno.EMFILE: logger.exception( "too many open files, please visit " "{} to see how to handle this " @@ -78,10 +78,13 @@ def main(argv=None): else: logger.exception("unexpected error") ret = 255 + except Exception: # noqa, pylint: disable=broad-except + logger.exception("unexpected error") + ret = 255 finally: logger.setLevel(outerLogLevel) - # Closing pools by-hand to prevent wierd messages when closing SSH + # Closing pools by-hand to prevent weird messages when closing SSH # connections. See https://github.com/iterative/dvc/issues/3248 for # more info. close_pools() diff --git a/dvc/output/base.py b/dvc/output/base.py index 97edd60fa2..4c7a99dcda 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -230,6 +230,8 @@ def isdir(self): def isfile(self): return self.remote.tree.isfile(self.path_info) + # pylint: disable=no-member + def ignore(self): if not self.use_scm_ignore: return @@ -245,6 +247,8 @@ def ignore_remove(self): self.repo.scm.ignore_remove(self.fspath) + # pylint: enable=no-member + def save(self): if not self.exists: raise self.DoesNotExistError(self) @@ -346,6 +350,7 @@ def remove(self, ignore_remove=False): self.ignore_remove() def move(self, out): + # pylint: disable=no-member if self.scheme == "local" and self.use_scm_ignore: self.repo.scm.ignore_remove(self.fspath) diff --git a/dvc/path_info.py b/dvc/path_info.py index e925a0a69c..2420764ba4 100644 --- a/dvc/path_info.py +++ b/dvc/path_info.py @@ -1,3 +1,4 @@ +# pylint: disable=protected-access import os import pathlib import posixpath @@ -17,7 +18,7 @@ def overlaps(self, other): return self.isin_or_eq(other) or other.isin(self) def isin_or_eq(self, other): - return self == other or self.isin(other) + return self == other or self.isin(other) # pylint: disable=no-member class PathInfo(pathlib.PurePath, _BasePath): @@ -37,7 +38,7 @@ def __new__(cls, *args): return cls._from_parts(args) def as_posix(self): - f = self._flavour + f = self._flavour # pylint: disable=no-member # Unlike original implementation [1] that uses `str()` we actually need # to use `fspath`, because we've overridden `__str__` method to return # relative paths, which will break original `as_posix`. @@ -264,7 +265,7 @@ def from_parts( params=None, query=None, fragment=None, - ): + ): # pylint: disable=arguments-differ assert bool(host) ^ bool(netloc) if netloc is not None: diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 1d8a285e74..b7d5cafa8e 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -100,7 +100,7 @@ class BaseRemoteTree: CACHE_MODE = None SHARED_MODE_MAP = {None: (None, None), "group": (None, None)} - CHECKSUM_DIR_SUFFIX = ".dir" + PARAM_CHECKSUM = None state = StateNoop() @@ -119,6 +119,7 @@ def __init__(self, repo, config): or self.HASH_JOBS ) self.verify = config.get("verify", self.DEFAULT_VERIFY) + self.path_info = None @classmethod def get_missing_deps(cls): @@ -184,7 +185,8 @@ def cache(self): def open(self, path_info, mode="r", encoding=None): if hasattr(self, "_generate_download_url"): - get_url = partial(self._generate_download_url, path_info) + func = self._generate_download_url # noqa,pylint:disable=no-member + get_url = partial(func, path_info) return open_url(get_url, mode=mode, encoding=encoding) raise RemoteActionNotImplemented("open", self.scheme) @@ -192,6 +194,8 @@ def open(self, path_info, mode="r", encoding=None): def exists(self, path_info): raise NotImplementedError + # pylint: disable=unused-argument + def isdir(self, path_info): """Optional: Overwrite only if the remote has a way to distinguish between a directory and a file. @@ -250,6 +254,8 @@ def protect(path_info): def is_protected(self, path_info): return False + # pylint: enable=unused-argument + @staticmethod def unprotect(path_info): pass @@ -413,7 +419,7 @@ def upload(self, from_info, to_info, name=None, no_progress_bar=False): name = name or from_info.name - self._upload( + self._upload( # noqa, pylint: disable=no-member from_info.fspath, to_info, name=name, @@ -504,7 +510,7 @@ def _download_file( tmp_file = tmp_fname(to_info) - self._download( + self._download( # noqa, pylint: disable=no-member from_info, tmp_file, name=name, no_progress_bar=no_progress_bar ) @@ -866,6 +872,7 @@ def gc(cls, named_cache, remote, jobs=None): path_info = tree.hash_to_path_info(hash_) if tree.is_dir_hash(hash_): # backward compatibility + # pylint: disable=protected-access tree._remove_unpacked_dir(hash_) tree.remove(path_info) removed = True diff --git a/dvc/remote/gs.py b/dvc/remote/gs.py index e20a3260e3..0775f0af51 100644 --- a/dvc/remote/gs.py +++ b/dvc/remote/gs.py @@ -33,6 +33,7 @@ def wrapper(*args, **kwargs): while True: try: # skipcq: PYL-W0212 + # pylint: disable=protected-access chunk_size = Blob._CHUNK_SIZE_MULTIPLE * multiplier return func(*args, chunk_size=chunk_size, **kwargs) except requests.exceptions.ConnectionError: @@ -102,7 +103,8 @@ def _generate_download_url(self, path_info, expires=3600): raise FileNotFoundError if isinstance( - blob.client._credentials, google.auth.credentials.Signing + blob.client._credentials, # pylint: disable=protected-access + google.auth.credentials.Signing, ): # sign if we're able to sign with credentials. return blob.generate_signed_url(expiration=expiration) diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 6e8e996b17..c691a78ab1 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -330,6 +330,7 @@ def wrapper(from_info, to_info, *args, **kwargs): except Exception as exc: # pylint: disable=broad-except # NOTE: this means we ran out of file descriptors and there is no # reason to try to proceed, as we will hit this error anyways. + # pylint: disable=no-member if isinstance(exc, OSError) and exc.errno == errno.EMFILE: raise @@ -362,7 +363,7 @@ def cache_dir(self, value): self.tree.path_info = PathInfo(value) if value else None @classmethod - def supported(cls, config): + def supported(cls, config): # pylint: disable=unused-argument return True @cached_property @@ -375,7 +376,9 @@ def hash_to_path(self, hash_): # being ~5.5 times faster. return f"{self.cache_path}{os.sep}{hash_[0:2]}{os.sep}{hash_[2:]}" - def hashes_exist(self, hashes, jobs=None, name=None): + def hashes_exist( + self, hashes, jobs=None, name=None + ): # pylint: disable=unused-argument return [ hash_ for hash_ in Tqdm( diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py index 110f177a13..d3559314bb 100644 --- a/dvc/remote/ssh/connection.py +++ b/dvc/remote/ssh/connection.py @@ -79,7 +79,7 @@ def getsize(self, path): return 0 - def exists(self, path, sftp=None): + def exists(self, path): return bool(self.st_mode(path)) def isdir(self, path): diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index d8bf59195d..685f179eeb 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -31,6 +31,7 @@ def locked(f): @wraps(f) def wrapper(repo, *args, **kwargs): with repo.lock, repo.state: + # pylint: disable=protected-access repo._reset() ret = f(repo, *args, **kwargs) # Our graph cache is no longer valid after we release the repo.lock diff --git a/dvc/repo/add.py b/dvc/repo/add.py index c1fee55d22..18983c6a95 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -147,7 +147,7 @@ def _create_stages(repo, targets, fname, pbar=None, external=False): if stage: Dvcfile(repo, stage.path).remove() - repo._reset() + repo._reset() # pylint: disable=protected-access if not stage: if pbar is not None: diff --git a/dvc/repo/gc.py b/dvc/repo/gc.py index 96ca40aece..ea86ab4f84 100644 --- a/dvc/repo/gc.py +++ b/dvc/repo/gc.py @@ -52,10 +52,9 @@ def gc( from contextlib import ExitStack from dvc.repo import Repo - all_repos = [] - - if repos: - all_repos = [Repo(path) for path in repos] + if not repos: + repos = [] + all_repos = [Repo(path) for path in repos] with ExitStack() as stack: for repo in all_repos: diff --git a/dvc/repo/plots/data.py b/dvc/repo/plots/data.py index 5f51e3757a..9ff23ccf90 100644 --- a/dvc/repo/plots/data.py +++ b/dvc/repo/plots/data.py @@ -181,7 +181,7 @@ def __init__(self, filename, revision, content, delimiter=","): super().__init__(filename, revision, content) self.delimiter = delimiter - def raw(self, header=True, **kwargs): + def raw(self, header=True, **kwargs): # pylint: disable=arguments-differ first_row = first(csv.reader(io.StringIO(self.content))) if header: diff --git a/dvc/repo/plots/template.py b/dvc/repo/plots/template.py index 1ea50abacd..a4e95df210 100644 --- a/dvc/repo/plots/template.py +++ b/dvc/repo/plots/template.py @@ -29,9 +29,13 @@ class Template: EXTENSION = ".json" ANCHOR = "" + DEFAULT_CONTENT = None + DEFAULT_NAME = None + def __init__(self, content=None, name=None): self.content = self.DEFAULT_CONTENT if content is None else content self.name = name or self.DEFAULT_NAME + assert self.content and self.name self.filename = self.name + self.EXTENSION def render(self, data, props=None): diff --git a/dvc/repo/pull.py b/dvc/repo/pull.py index 4ee782bad1..6e62652ca8 100644 --- a/dvc/repo/pull.py +++ b/dvc/repo/pull.py @@ -22,7 +22,7 @@ def pull( if isinstance(targets, str): targets = [targets] - processed_files_count = self._fetch( + processed_files_count = self._fetch( # pylint: disable=protected-access targets, jobs, remote=remote, @@ -33,7 +33,7 @@ def pull( recursive=recursive, run_cache=run_cache, ) - stats = self._checkout( + stats = self._checkout( # pylint: disable=protected-access targets=targets, with_deps=with_deps, force=force, recursive=recursive ) diff --git a/dvc/repo/tree.py b/dvc/repo/tree.py index 23cf58a0fa..a23f96bbc3 100644 --- a/dvc/repo/tree.py +++ b/dvc/repo/tree.py @@ -55,7 +55,9 @@ def _get_granular_checksum(self, path, out, remote=None): return entry[out.remote.tree.PARAM_CHECKSUM] raise FileNotFoundError - def open(self, path, mode="r", encoding="utf-8", remote=None): + def open( + self, path, mode="r", encoding="utf-8", remote=None + ): # pylint: disable=arguments-differ try: outs = self._find_outs(path, strict=False) except OutputNotFoundError as exc: @@ -221,7 +223,7 @@ def isdvc(self, path, **kwargs): pass return False - def isexec(self, path): + def isexec(self, path): # pylint: disable=unused-argument return False def get_file_hash(self, path_info): @@ -345,7 +347,9 @@ def _walk(self, dvc_walk, repo_walk, dvcfiles=False): elif dirname in repo_set: yield from self._walk_one(repo_walk) - def walk(self, top, topdown=True, onerror=None, dvcfiles=False, **kwargs): + def walk( + self, top, topdown=True, onerror=None, dvcfiles=False, **kwargs + ): # pylint: disable=arguments-differ """Walk and merge both DVC and repo trees. Args: diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 055b9dba80..8056d4177c 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -119,7 +119,7 @@ def clone(url, to_path, rev=None): progress=pbar.update_git, ) tmp_repo.close() - except git.exc.GitCommandError as exc: + except git.exc.GitCommandError as exc: # pylint: disable=no-member raise CloneError(url, to_path) from exc # NOTE: using our wrapper to make sure that env is fixed in __init__ @@ -128,7 +128,7 @@ def clone(url, to_path, rev=None): if rev: try: repo.checkout(rev) - except git.exc.GitCommandError as exc: + except git.exc.GitCommandError as exc: # pylint: disable=no-member raise RevError( "failed to access revision '{}' for repo '{}'".format( rev, url diff --git a/dvc/scm/git/tree.py b/dvc/scm/git/tree.py index 2ea999c66c..5b3197866d 100644 --- a/dvc/scm/git/tree.py +++ b/dvc/scm/git/tree.py @@ -94,7 +94,7 @@ def _git_object_by_path(self, path): try: tree = self.git.tree(self.rev) - except git.exc.BadName as exc: + except git.exc.BadName as exc: # pylint: disable=no-member raise DvcException( "revision '{}' not found in Git '{}'".format( self.rev, os.path.relpath(self.git.working_dir) diff --git a/dvc/stage/decorators.py b/dvc/stage/decorators.py index 1ce98e5bbb..3623fb5251 100644 --- a/dvc/stage/decorators.py +++ b/dvc/stage/decorators.py @@ -15,7 +15,7 @@ def rwlocked(call, read=None, write=None): if write is None: write = [] - stage = call._args[0] + stage = call._args[0] # pylint: disable=protected-access assert stage.repo.lock.is_locked @@ -40,7 +40,7 @@ def unlocked_repo(f): def wrapper(stage, *args, **kwargs): stage.repo.state.dump() stage.repo.lock.unlock() - stage.repo._reset() + stage.repo._reset() # pylint: disable=protected-access try: ret = f(stage, *args, **kwargs) finally: diff --git a/dvc/stage/loader.py b/dvc/stage/loader.py index c23d9d6484..246733d353 100644 --- a/dvc/stage/loader.py +++ b/dvc/stage/loader.py @@ -125,7 +125,7 @@ def __getitem__(self, item): def load_stage(cls, dvcfile, d, stage_text): path, wdir = resolve_paths(dvcfile.path, d.get(Stage.PARAM_WDIR)) stage = loads_from(Stage, dvcfile.repo, path, wdir, d) - stage._stage_text = stage_text + stage._stage_text = stage_text # noqa, pylint:disable=protected-access stage.deps = dependency.loadd_from( stage, d.get(Stage.PARAM_DEPS) or [] ) diff --git a/dvc/stage/run.py b/dvc/stage/run.py index 53bab38d21..2cd30c5f67 100644 --- a/dvc/stage/run.py +++ b/dvc/stage/run.py @@ -60,7 +60,10 @@ def cmd_run(stage, *args, **kwargs): warn_if_fish(executable) cmd = _nix_cmd(executable, stage.cmd) - main_thread = isinstance(threading.current_thread(), threading._MainThread) + main_thread = isinstance( + threading.current_thread(), + threading._MainThread, # pylint: disable=protected-access + ) old_handler = None p = None diff --git a/dvc/stage/serialize.py b/dvc/stage/serialize.py index eaea052926..44d3c4f3bd 100644 --- a/dvc/stage/serialize.py +++ b/dvc/stage/serialize.py @@ -173,8 +173,9 @@ def to_single_stage_file(stage: "Stage"): # - reparse the same yaml text with a slow but smart ruamel yaml parser # - apply changes to a returned structure # - serialize it - if stage._stage_text is not None: - saved_state = parse_yaml_for_update(stage._stage_text, stage.path) + text = stage._stage_text # noqa, pylint: disable=protected-access + if text is not None: + saved_state = parse_yaml_for_update(text, stage.path) # Stage doesn't work with meta in any way, so .dumpd() doesn't # have it. We simply copy it over. if "meta" in saved_state: diff --git a/dvc/state.py b/dvc/state.py index c634b2d3a4..257c09fd10 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -35,7 +35,7 @@ class StateNoop: def save(self, path_info, checksum): pass - def get(self, path_info): + def get(self, path_info): # pylint: disable=unused-argument return None def save_link(self, path_info): diff --git a/dvc/system.py b/dvc/system.py index aeafd3ea0d..3439f41198 100644 --- a/dvc/system.py +++ b/dvc/system.py @@ -77,7 +77,7 @@ def _reflink_darwin(src, dst): ) @staticmethod - def _reflink_windows(src, dst): + def _reflink_windows(_src, _dst): return -1 @staticmethod diff --git a/dvc/utils/fs.py b/dvc/utils/fs.py index 553bbe76c6..b48d0af4a9 100644 --- a/dvc/utils/fs.py +++ b/dvc/utils/fs.py @@ -108,7 +108,7 @@ def move(src, dst, mode=None): shutil.move(tmp, dst) -def _chmod(func, p, excinfo): +def _chmod(func, p, excinfo): # pylint: disable=unused-argument perm = os.lstat(p).st_mode perm |= stat.S_IWRITE diff --git a/fastentrypoints.py b/fastentrypoints.py index 3325cd7ea8..fb4a8646e1 100644 --- a/fastentrypoints.py +++ b/fastentrypoints.py @@ -79,7 +79,7 @@ def get_args(cls, dist, header=None): # noqa: D205,D400 group, name, ) - # pylint: disable=E1101 + # pylint: disable=E1101,protected-access args = cls._get_script_args(type_, name, header, script_text) for res in args: yield res diff --git a/setup.cfg b/setup.cfg index f7cb136c8c..6e9dd89a42 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,7 +17,7 @@ count=true [isort] include_trailing_comma=true known_first_party=dvc,tests -known_third_party=PyInstaller,RangeHTTPServer,boto3,colorama,configobj,distro,dpath,flaky,flufl,funcy,git,google,grandalf,mock,moto,nanotime,networkx,packaging,paramiko,pathspec,pytest,requests,ruamel,setuptools,shortuuid,shtab,tqdm,voluptuous,yaml,zc +known_third_party=PyInstaller,RangeHTTPServer,boto3,colorama,configobj,distro,dpath,flaky,flufl,funcy,git,google,grandalf,mock,moto,nanotime,networkx,packaging,paramiko,pathspec,pylint,pytest,requests,ruamel,setuptools,shortuuid,shtab,tqdm,voluptuous,yaml,zc line_length=79 force_grid_wrap=0 use_parentheses=True diff --git a/setup.py b/setup.py index 537a7dc38c..ab63ab0c16 100644 --- a/setup.py +++ b/setup.py @@ -129,6 +129,7 @@ def run(self): "flake8-string-format", "pylint", "pylint-pytest", + "pylint-plugin-utils", ] if (sys.version_info) >= (3, 6): diff --git a/tests/func/test_api.py b/tests/func/test_api.py index aee81b56d0..d1b67f58e2 100644 --- a/tests/func/test_api.py +++ b/tests/func/test_api.py @@ -23,9 +23,11 @@ remotes = [pytest.lazy_fixture(f"{cloud}_remote") for cloud in cloud_names] all_remotes = [pytest.lazy_fixture("local_remote")] + remotes +# `lazy_fixture` is confusing pylint, pylint: disable=unused-argument + @pytest.mark.parametrize("remote", remotes) -def test_get_url(tmp_dir, dvc, request, remote): +def test_get_url(tmp_dir, dvc, remote): tmp_dir.dvc_gen("foo", "foo") expected_url = URLInfo(remote.url) / "ac/bd18db4cc2f85cedef654fccc4a4d8" diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py index c34b3a9f07..c3cdb04fb0 100644 --- a/tests/func/test_checkout.py +++ b/tests/func/test_checkout.py @@ -383,7 +383,7 @@ def test(self): class TestCheckoutHook(TestDvc): @patch("sys.stdout.isatty", return_value=True) @patch("dvc.prompt.input", side_effect=EOFError) - def test(self, mock_input, mock_isatty): + def test(self, _mock_input, _mock_isatty): """ Test that dvc checkout handles EOFError gracefully, which is what it will experience when running in a git hook. """ @@ -400,6 +400,7 @@ def test(self, mock_input, mock_isatty): class TestCheckoutSuggestGit(TestRepro): def test(self): + # pylint: disable=no-member try: self.dvc.checkout(targets="gitbranch") diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 6f87e0b809..d0d6fca0da 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -18,7 +18,7 @@ @pytest.mark.parametrize("remote", all_remotes) -def test_cloud(tmp_dir, dvc, remote): +def test_cloud(tmp_dir, dvc, remote): # pylint:disable=unused-argument (stage,) = tmp_dir.dvc_gen("foo", "foo") out = stage.outs[0] cache = out.cache_path diff --git a/tests/func/test_dvcfile.py b/tests/func/test_dvcfile.py index 849b029777..8c7978d648 100644 --- a/tests/func/test_dvcfile.py +++ b/tests/func/test_dvcfile.py @@ -1,3 +1,4 @@ +# pylint: disable=no-member import textwrap import pytest diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 64e5a66f7f..9b77a0f727 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -52,7 +52,7 @@ class SingleStageRun: def _run(self, **kwargs): kwargs["single_stage"] = True kwargs.pop("name", None) - return self.dvc.run(**kwargs) + return self.dvc.run(**kwargs) # noqa, pylint: disable=no-member @staticmethod def _get_stage_target(stage): @@ -841,7 +841,7 @@ def test(self): self.assertTrue(filecmp.cmp(foo, bar, shallow=False)) -class TestReproExternalBase(SingleStageRun, TestDvc): +class ReproExternalTestMixin(SingleStageRun, TestDvc): cache_type = None @staticmethod @@ -892,7 +892,7 @@ def check_already_cached(self, stage): mock_checkout.assert_called_once() @patch("dvc.prompt.confirm", return_value=True) - def test(self, mock_prompt): + def test(self, _mock_prompt): if not self.should_test(): raise SkipTest(f"Test {self.__class__.__name__} is disabled") @@ -998,7 +998,7 @@ def test(self, mock_prompt): @pytest.mark.skipif(os.name == "nt", reason="temporarily disabled on windows") @flaky(max_runs=3, min_passes=1) -class TestReproExternalS3(S3, TestReproExternalBase): +class TestReproExternalS3(S3, ReproExternalTestMixin): @property def scheme(self): return "s3" @@ -1015,7 +1015,7 @@ def write(self, bucket, key, body): s3.put_object(Bucket=bucket, Key=key, Body=body) -class TestReproExternalGS(GCP, TestReproExternalBase): +class TestReproExternalGS(GCP, ReproExternalTestMixin): @property def scheme(self): return "gs" @@ -1033,7 +1033,7 @@ def write(self, bucket, key, body): bucket.blob(key).upload_from_string(body) -class TestReproExternalHDFS(HDFS, TestReproExternalBase): +class TestReproExternalHDFS(HDFS, ReproExternalTestMixin): @property def scheme(self): return "hdfs" @@ -1090,7 +1090,7 @@ def write(self, bucket, key, body): @flaky(max_runs=5, min_passes=1) -class TestReproExternalSSH(SSH, TestReproExternalBase): +class TestReproExternalSSH(SSH, ReproExternalTestMixin): _dir = None cache_type = "copy" @@ -1111,7 +1111,7 @@ def cmd(self, i, o): o = o[len(prefix) :] return f"scp {i} {o}" - def write(self, bucket, key, body): + def write(self, _, key, body): path = posixpath.join(self._dir, key) ssh = None @@ -1141,7 +1141,7 @@ def write(self, bucket, key, body): ssh.close() -class TestReproExternalLOCAL(Local, TestReproExternalBase): +class TestReproExternalLOCAL(Local, ReproExternalTestMixin): cache_type = "hardlink" def setUp(self): @@ -1187,7 +1187,7 @@ def write(self, bucket, key, body): fd.write(body) -class TestReproExternalHTTP(TestReproExternalBase): +class TestReproExternalHTTP(ReproExternalTestMixin): _external_cache_id = None @staticmethod @@ -1198,7 +1198,7 @@ def get_remote(port): def local_cache(self): return os.path.join(self.dvc.dvc_dir, "cache") - def test(self): + def test(self): # pylint: disable=arguments-differ # Import with StaticFileServer() as httpd: import_url = urljoin(self.get_remote(httpd.server_port), self.FOO) diff --git a/tests/func/test_repro_multistage.py b/tests/func/test_repro_multistage.py index 4a9eabca42..995cc409df 100644 --- a/tests/func/test_repro_multistage.py +++ b/tests/func/test_repro_multistage.py @@ -28,7 +28,7 @@ def _run(self, **kwargs): assert kwargs.get("name") kwargs.pop("fname", None) # ignore fname for now - return self.dvc.run(**kwargs) + return self.dvc.run(**kwargs) # noqa, pylint: disable=no-member @staticmethod def _get_stage_target(stage): diff --git a/tests/func/test_state.py b/tests/func/test_state.py index 616faf672f..f9c5469407 100644 --- a/tests/func/test_state.py +++ b/tests/func/test_state.py @@ -47,7 +47,7 @@ def test_state_overflow(tmp_dir, dvc): def mock_get_inode(inode): - def get_inode_mocked(path): + def get_inode_mocked(_): return inode return get_inode_mocked diff --git a/tests/func/test_tree.py b/tests/func/test_tree.py index bdfe7c71b2..ebfd9aea69 100644 --- a/tests/func/test_tree.py +++ b/tests/func/test_tree.py @@ -40,6 +40,7 @@ def test_isfile(self): class GitTreeTests: + # pylint: disable=no-member def test_open(self): self.scm.add([self.FOO, self.UNICODE, self.DATA_DIR]) self.scm.commit("add") diff --git a/tests/pylint_plugin_disable.py b/tests/pylint_plugin_disable.py new file mode 100644 index 0000000000..684e572e32 --- /dev/null +++ b/tests/pylint_plugin_disable.py @@ -0,0 +1,47 @@ +""" +Disable certain checks for the tests. + +To disable new checks, add to the `SUPPRESS_CHECKS` dictionary, +with `message_id` of the check to disable as a key and a list of +methods that check for that particular message. +""" +import os.path +from typing import TYPE_CHECKING + +from pylint.checkers.base import NameChecker +from pylint.checkers.classes import ClassChecker + +if TYPE_CHECKING: + from astroid import node_classes, scoped_nodes # noqa + from pylint.lint import PyLinter + + +TESTS_FOLDER = os.path.join(os.path.abspath(os.path.dirname(__file__)), "") +SUPPRESS_CHECKS = { + "protected-access": [ + ClassChecker.visit_assign, + ClassChecker.visit_attribute, + ], + "blacklisted-name": [ + NameChecker.visit_global, + NameChecker.visit_assignname, + ], +} + + +def is_node_in_tests(node: "node_classes.NodeNG"): + module: "scoped_nodes.Module" = node.root() + return module.file.startswith(TESTS_FOLDER) + + +def register(linter: "PyLinter"): # noqa + try: + from pylint_plugin_utils import suppress_message + except ImportError: + print("Cannot suppress message. 'pylint_plugin_utils' not installed.") + return + + print("Registered custom plugin. Some checks will be disabled for tests.") + for msg, checks in SUPPRESS_CHECKS.items(): + for checker_method in checks: + suppress_message(linter, checker_method, msg, is_node_in_tests) diff --git a/tests/remotes/http.py b/tests/remotes/http.py index 0545932ccc..49cf75ffb1 100644 --- a/tests/remotes/http.py +++ b/tests/remotes/http.py @@ -5,7 +5,7 @@ class HTTP(Base): @staticmethod - def get_url(port): + def get_url(port): # pylint: disable=arguments-differ return f"http://127.0.0.1:{port}" def __init__(self, server): diff --git a/tests/remotes/ssh.py b/tests/remotes/ssh.py index c0514b363f..85387b59ae 100644 --- a/tests/remotes/ssh.py +++ b/tests/remotes/ssh.py @@ -43,7 +43,7 @@ def get_url(): class SSHMocked(Base): @staticmethod - def get_url(user, port): + def get_url(user, port): # pylint: disable=arguments-differ path = Local.get_storagepath() if os.name == "nt": # NOTE: On Windows Local.get_storagepath() will return an diff --git a/tests/unit/output/test_local.py b/tests/unit/output/test_local.py index c4b214ecec..0cf50869b2 100644 --- a/tests/unit/output/test_local.py +++ b/tests/unit/output/test_local.py @@ -83,7 +83,7 @@ def test_return_0_on_no_cache(self): "get_dir_cache", return_value=[{"md5": "asdf"}, {"md5": "qwe"}], ) - def test_return_multiple_for_dir(self, mock_get_dir_cache): + def test_return_multiple_for_dir(self, _mock_get_dir_cache): o = self._get_output() self.assertEqual(2, o.get_files_number()) diff --git a/tests/unit/remote/test_base.py b/tests/unit/remote/test_base.py index 1d6bbe4001..c2025f1c24 100644 --- a/tests/unit/remote/test_base.py +++ b/tests/unit/remote/test_base.py @@ -106,7 +106,7 @@ def test_hashes_exist(object_exists, traverse, dvc): @mock.patch.object( BaseRemoteTree, "path_to_hash", side_effect=lambda x: x, ) -def test_list_hashes_traverse(path_to_hash, list_hashes, dvc): +def test_list_hashes_traverse(_path_to_hash, list_hashes, dvc): tree = BaseRemoteTree(dvc, {}) tree.path_info = PathInfo("foo") diff --git a/tests/unit/remote/test_slow_link_detection.py b/tests/unit/remote/test_slow_link_detection.py index 6100a59690..01491d9700 100644 --- a/tests/unit/remote/test_slow_link_detection.py +++ b/tests/unit/remote/test_slow_link_detection.py @@ -27,8 +27,10 @@ def test_show_warning_once(caplog, make_remote): slow_link_guard(lambda x: None)(remote) slow_link_guard(lambda x: None)(remote) + slow_link_detection = dvc.remote.slow_link_detection + message = slow_link_detection.message # noqa, pylint: disable=no-member assert len(caplog.records) == 1 - assert caplog.records[0].message == dvc.remote.slow_link_detection.message + assert caplog.records[0].message == message def test_dont_warn_when_cache_type_is_set(caplog, make_remote): diff --git a/tests/unit/stage/test_stage.py b/tests/unit/stage/test_stage.py index 3ca83d9d95..28135a5ec1 100644 --- a/tests/unit/stage/test_stage.py +++ b/tests/unit/stage/test_stage.py @@ -76,7 +76,10 @@ def test_stage_update(mocker): @pytest.mark.skipif( - not isinstance(threading.current_thread(), threading._MainThread), + not isinstance( + threading.current_thread(), + threading._MainThread, # noqa, pylint: disable=protected-access + ), reason="Not running in the main thread.", ) def test_stage_run_ignore_sigint(dvc, mocker):