Skip to content

Commit

Permalink
pylint: enable additional checks (iterative#4122)
Browse files Browse the repository at this point in the history
Created a separate pylint plugin to disable some checks for tests.
  • Loading branch information
skshetry authored Jun 29, 2020
1 parent a7e825a commit 1c0f5cd
Show file tree
Hide file tree
Showing 50 changed files with 175 additions and 78 deletions.
20 changes: 9 additions & 11 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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).
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dvc/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions dvc/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion dvc/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions dvc/command/git_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ def run(self):

return self._run()

def _run(self):
raise NotImplementedError


class CmdPreCommit(CmdHookBase):
def _run(self):
Expand Down
8 changes: 5 additions & 3 deletions dvc/dvcfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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()


Expand All @@ -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

Expand Down
1 change: 1 addition & 0 deletions dvc/external_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def clean_repos():


class BaseExternalRepo:
# pylint: disable=no-member

_local_cache = None

Expand Down
2 changes: 1 addition & 1 deletion dvc/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions dvc/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
9 changes: 6 additions & 3 deletions dvc/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand All @@ -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()
Expand Down
5 changes: 5 additions & 0 deletions dvc/output/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand Down
7 changes: 4 additions & 3 deletions dvc/path_info.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: disable=protected-access
import os
import pathlib
import posixpath
Expand All @@ -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):
Expand All @@ -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`.
Expand Down Expand Up @@ -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:
Expand Down
15 changes: 11 additions & 4 deletions dvc/remote/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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):
Expand Down Expand Up @@ -184,14 +185,17 @@ 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)

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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion dvc/remote/gs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions dvc/remote/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion dvc/remote/ssh/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions dvc/repo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dvc/repo/add.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 1c0f5cd

Please sign in to comment.