From 0a13d4a127479e22c14e78d06f6c0e4343c84a96 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 15 Jan 2024 12:08:31 -0800 Subject: [PATCH 1/7] Add ruff pre-commit hook --- .pre-commit-config.yaml | 7 +++++++ pyproject.toml | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 29934dd11..cda71e761 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -16,6 +16,13 @@ repos: rev: 22.3.0 hooks: - id: black + - repo: https://github.com/astral-sh/ruff-pre-commit + # Ruff version. + rev: v0.1.13 + hooks: + # Run the linter. + - id: ruff + args: [ --fix ] - repo: https://github.com/PyCQA/flake8 rev: 5.0.4 hooks: diff --git a/pyproject.toml b/pyproject.toml index ab9055633..59adbf39c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,29 @@ exclude = ''' ) ''' +[tool.ruff] +exclude = [ + ".tox", + "build", + "docs/source/conf.py", + "versioneer.py", + "fsspec/_version", +] +line-length = 88 +ignore = [ + # Assigning lambda expression + "E731", + # Ambiguous variable names + "E741", + # line break before binary operator + # uncomment when implemented in ruff + # "W503", + # whitespace before : + "E203", + # redefs + "F811", +] + [tool.pytest.ini_options] # custom markers, need to be defined to avoid warnings markers = [ From a0d0115df98e88475b8a5a006905860e9c14519b Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 15 Jan 2024 12:29:02 -0800 Subject: [PATCH 2/7] Enable way more rules and add fixes or noqas --- fsspec/exceptions.py | 4 --- fsspec/gui.py | 5 ++-- fsspec/implementations/cache_mapper.py | 8 ++--- fsspec/implementations/http.py | 6 ++-- fsspec/implementations/tests/test_cached.py | 2 +- fsspec/implementations/tests/test_dirfs.py | 2 +- fsspec/implementations/tests/test_http.py | 2 +- .../implementations/tests/test_reference.py | 4 +-- fsspec/implementations/tests/test_tar.py | 2 +- fsspec/implementations/tests/test_zip.py | 2 +- fsspec/parquet.py | 6 ++-- fsspec/spec.py | 4 ++- fsspec/tests/test_fuse.py | 1 + fsspec/tests/test_spec.py | 1 + pyproject.toml | 30 +++++++++++++++++++ 15 files changed, 52 insertions(+), 27 deletions(-) diff --git a/fsspec/exceptions.py b/fsspec/exceptions.py index 2d6e1a44b..0593f0e58 100644 --- a/fsspec/exceptions.py +++ b/fsspec/exceptions.py @@ -10,12 +10,8 @@ class BlocksizeMismatchError(ValueError): written with """ - ... - class FSTimeoutError(asyncio.TimeoutError): """ Raised when a fsspec function timed out occurs """ - - ... diff --git a/fsspec/gui.py b/fsspec/gui.py index efa5b5f86..14fc4935c 100644 --- a/fsspec/gui.py +++ b/fsspec/gui.py @@ -153,8 +153,9 @@ def _emit(self, sig, value=None): break except Exception as e: logger.exception( - "Exception (%s) while executing callback for signal: %s" - "" % (e, sig) + "Exception (%s) while executing callback for signal: %s", + e, + sig, ) def show(self, threads=False): diff --git a/fsspec/implementations/cache_mapper.py b/fsspec/implementations/cache_mapper.py index 000ccebc8..64cc90880 100644 --- a/fsspec/implementations/cache_mapper.py +++ b/fsspec/implementations/cache_mapper.py @@ -2,13 +2,9 @@ import abc import hashlib -from typing import TYPE_CHECKING from fsspec.implementations.local import make_path_posix -if TYPE_CHECKING: - from typing import Any - class AbstractCacheMapper(abc.ABC): """Abstract super-class for mappers from remote URLs to local cached @@ -19,7 +15,7 @@ class AbstractCacheMapper(abc.ABC): def __call__(self, path: str) -> str: ... - def __eq__(self, other: Any) -> bool: + def __eq__(self, other: object) -> bool: # Identity only depends on class. When derived classes have attributes # they will need to be included. return isinstance(other, type(self)) @@ -56,7 +52,7 @@ def __call__(self, path: str) -> str: else: return prefix # No separator found, simple filename - def __eq__(self, other: Any) -> bool: + def __eq__(self, other: object) -> bool: return super().__eq__(other) and self.directory_levels == other.directory_levels def __hash__(self) -> int: diff --git a/fsspec/implementations/http.py b/fsspec/implementations/http.py index d1bb013b6..e7fef2f1b 100644 --- a/fsspec/implementations/http.py +++ b/fsspec/implementations/http.py @@ -251,7 +251,7 @@ async def _get_file( if isfilelike(lpath): outfile = lpath else: - outfile = open(lpath, "wb") + outfile = open(lpath, "wb") # noqa: ASYNC101 try: chunk = True @@ -279,7 +279,7 @@ async def gen_chunks(): context = nullcontext(lpath) use_seek = False # might not support seeking else: - context = open(lpath, "rb") + context = open(lpath, "rb") # noqa: ASYNC101 use_seek = True with context as f: @@ -801,7 +801,7 @@ async def get_range(session, url, start, end, file=None, **kwargs): async with r: out = await r.read() if file: - with open(file, "r+b") as f: + with open(file, "r+b") as f: # noqa: ASYNC101 f.seek(start) f.write(out) else: diff --git a/fsspec/implementations/tests/test_cached.py b/fsspec/implementations/tests/test_cached.py index ae641bcc0..05baf5d97 100644 --- a/fsspec/implementations/tests/test_cached.py +++ b/fsspec/implementations/tests/test_cached.py @@ -1022,7 +1022,7 @@ def test_multi_cache(protocol): def test_multi_cat(protocol, ftp_writable): host, port, user, pw = ftp_writable fs = FTPFileSystem(host, port, user, pw) - for fn in {"/file0", "/file1"}: + for fn in ("/file0", "/file1"): with fs.open(fn, "wb") as f: f.write(b"hello") diff --git a/fsspec/implementations/tests/test_dirfs.py b/fsspec/implementations/tests/test_dirfs.py index b2a7bbebb..e42b9b696 100644 --- a/fsspec/implementations/tests/test_dirfs.py +++ b/fsspec/implementations/tests/test_dirfs.py @@ -355,7 +355,7 @@ async def _walk(path, *args, **kwargs): actual = [] async for entry in adirfs._walk("root", *ARGS, **KWARGS): - actual.append(entry) + actual.append(entry) # noqa: PERF402 assert actual == [("root", ["foo", "bar"], ["baz", "qux"])] adirfs.fs._walk.assert_called_once_with(f"{PATH}/root", *ARGS, **KWARGS) diff --git a/fsspec/implementations/tests/test_http.py b/fsspec/implementations/tests/test_http.py index 6f0281bc2..af1164480 100644 --- a/fsspec/implementations/tests/test_http.py +++ b/fsspec/implementations/tests/test_http.py @@ -557,7 +557,7 @@ async def test_async_walk(server): # No maxdepth res = [] async for a in fs._walk(server + "/index"): - res.append(a) + res.append(a) # noqa: PERF402 assert res == [(server + "/index", [], ["realfile"])] # maxdepth=0 diff --git a/fsspec/implementations/tests/test_reference.py b/fsspec/implementations/tests/test_reference.py index 19bc0cfa4..6a0c9ab55 100644 --- a/fsspec/implementations/tests/test_reference.py +++ b/fsspec/implementations/tests/test_reference.py @@ -61,9 +61,9 @@ def test_nested_dirs_ls(): refs = {"a": "A", "B/C/b": "B", "B/C/d": "d", "B/_": "_"} fs = fsspec.filesystem("reference", fo=refs) assert len(fs.ls("")) == 2 - assert set(e["name"] for e in fs.ls("")) == set(["a", "B"]) + assert {e["name"] for e in fs.ls("")} == {"a", "B"} assert len(fs.ls("B")) == 2 - assert set(e["name"] for e in fs.ls("B")) == set(["B/C", "B/_"]) + assert {e["name"] for e in fs.ls("B")} == {"B/C", "B/_"} def test_info(server): # noqa: F811 diff --git a/fsspec/implementations/tests/test_tar.py b/fsspec/implementations/tests/test_tar.py index 23a4c2ea5..d55e94fc8 100644 --- a/fsspec/implementations/tests/test_tar.py +++ b/fsspec/implementations/tests/test_tar.py @@ -42,7 +42,7 @@ def test_info(): assert lhs == expected # Iterate over all files. - for f, v in archive_data.items(): + for f in archive_data.keys(): lhs = fs.info(f) # Probe some specific fields of Tar archives. diff --git a/fsspec/implementations/tests/test_zip.py b/fsspec/implementations/tests/test_zip.py index e6bc938bf..842d01673 100644 --- a/fsspec/implementations/tests/test_zip.py +++ b/fsspec/implementations/tests/test_zip.py @@ -12,7 +12,7 @@ def test_info(): fs = fsspec.filesystem("zip", fo=z) # Iterate over all files. - for f, v in archive_data.items(): + for f in archive_data.keys(): lhs = fs.info(f) # Probe some specific fields of Zip archives. diff --git a/fsspec/parquet.py b/fsspec/parquet.py index af55f8cf4..c46410d00 100644 --- a/fsspec/parquet.py +++ b/fsspec/parquet.py @@ -131,10 +131,8 @@ def open_parquet_file( cache_type="parts", cache_options={ **options, - **{ - "data": data.get(fn, {}), - "strict": strict, - }, + "data": data.get(fn, {}), + "strict": strict, }, **kwargs, ) diff --git a/fsspec/spec.py b/fsspec/spec.py index ed7ee4006..39ee91a16 100644 --- a/fsspec/spec.py +++ b/fsspec/spec.py @@ -1398,7 +1398,9 @@ def to_json(self): ) return json.dumps( dict( - **{"cls": cls, "protocol": proto, "args": self.storage_args}, + cls=cls, + protocol=proto, + args=self.storage_args, **self.storage_options, ) ) diff --git a/fsspec/tests/test_fuse.py b/fsspec/tests/test_fuse.py index 408ef5f82..db627ffc9 100644 --- a/fsspec/tests/test_fuse.py +++ b/fsspec/tests/test_fuse.py @@ -123,6 +123,7 @@ def test_chmod(mount_local): ["cp", str(mount_dir / "text"), str(mount_dir / "new")], stdout=subprocess.PIPE, stderr=subprocess.PIPE, + check=False, ) assert cp.stderr == b"" diff --git a/fsspec/tests/test_spec.py b/fsspec/tests/test_spec.py index 69ab86045..6ab03abe5 100644 --- a/fsspec/tests/test_spec.py +++ b/fsspec/tests/test_spec.py @@ -1147,6 +1147,7 @@ def test_posix_tests_bash_stat(path, expected, glob_files_folder): f"cd {glob_files_folder} && shopt -s globstar && stat -c %N {bash_path}", ], capture_output=True, + check=False, ) # Remove the last element always empty bash_output = bash_output.stdout.decode("utf-8").replace("'", "").split("\n")[:-1] diff --git a/pyproject.toml b/pyproject.toml index 59adbf39c..b556c2657 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,6 +30,32 @@ exclude = [ "versioneer.py", "fsspec/_version", ] +select = [ + # fix noqas in fsspec/implementations/http.py + "ASYNC", + # "B", enable in later PR + "C4", + "G", + "E4", + "E7", + "E9", + "F", + "PERF", + "PLC", + "PLE", + "PLR1722", + "PLW1510", + "PLW3301", + "PIE800", + "PIE804", + "PIE807", + "PIE810", + # "PT", enable in later PR + "PYI", + "RUF006", + "SLOT", + "SIM101", +] line-length = 88 ignore = [ # Assigning lambda expression @@ -43,6 +69,10 @@ ignore = [ "E203", # redefs "F811", + # Fix these codes later + "G004", + "PERF203", + "PERF401", ] [tool.pytest.ini_options] From 84b2bddbff3c7f94403e1cf2986503ed0ad982c9 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 16 Jan 2024 11:39:37 -0500 Subject: [PATCH 3/7] Remove keys() from for loops Co-authored-by: Martin Durant --- fsspec/implementations/tests/test_tar.py | 2 +- fsspec/implementations/tests/test_zip.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fsspec/implementations/tests/test_tar.py b/fsspec/implementations/tests/test_tar.py index d55e94fc8..0ec7c8a6e 100644 --- a/fsspec/implementations/tests/test_tar.py +++ b/fsspec/implementations/tests/test_tar.py @@ -42,7 +42,7 @@ def test_info(): assert lhs == expected # Iterate over all files. - for f in archive_data.keys(): + for f in archive_data: lhs = fs.info(f) # Probe some specific fields of Tar archives. diff --git a/fsspec/implementations/tests/test_zip.py b/fsspec/implementations/tests/test_zip.py index 842d01673..ec30c8778 100644 --- a/fsspec/implementations/tests/test_zip.py +++ b/fsspec/implementations/tests/test_zip.py @@ -12,7 +12,7 @@ def test_info(): fs = fsspec.filesystem("zip", fo=z) # Iterate over all files. - for f in archive_data.keys(): + for f in archive_data: lhs = fs.info(f) # Probe some specific fields of Zip archives. From cd48c8417b6d4dbfa9fcbdfe4043c4409c443f5b Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 16 Jan 2024 09:33:13 -0800 Subject: [PATCH 4/7] Change to check=True for subprocess --- fsspec/tests/test_fuse.py | 2 +- fsspec/tests/test_spec.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fsspec/tests/test_fuse.py b/fsspec/tests/test_fuse.py index db627ffc9..6794cc086 100644 --- a/fsspec/tests/test_fuse.py +++ b/fsspec/tests/test_fuse.py @@ -123,7 +123,7 @@ def test_chmod(mount_local): ["cp", str(mount_dir / "text"), str(mount_dir / "new")], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - check=False, + check=True, ) assert cp.stderr == b"" diff --git a/fsspec/tests/test_spec.py b/fsspec/tests/test_spec.py index 6ab03abe5..2f5ed73f1 100644 --- a/fsspec/tests/test_spec.py +++ b/fsspec/tests/test_spec.py @@ -1147,7 +1147,7 @@ def test_posix_tests_bash_stat(path, expected, glob_files_folder): f"cd {glob_files_folder} && shopt -s globstar && stat -c %N {bash_path}", ], capture_output=True, - check=False, + check=True, ) # Remove the last element always empty bash_output = bash_output.stdout.decode("utf-8").replace("'", "").split("\n")[:-1] From bc27fb24efd4316f929424e5f90c3dd710223c93 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 16 Jan 2024 09:45:07 -0800 Subject: [PATCH 5/7] Revert "Change to check=True for subprocess" This reverts commit cd48c8417b6d4dbfa9fcbdfe4043c4409c443f5b. --- fsspec/tests/test_fuse.py | 2 +- fsspec/tests/test_spec.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fsspec/tests/test_fuse.py b/fsspec/tests/test_fuse.py index 6794cc086..db627ffc9 100644 --- a/fsspec/tests/test_fuse.py +++ b/fsspec/tests/test_fuse.py @@ -123,7 +123,7 @@ def test_chmod(mount_local): ["cp", str(mount_dir / "text"), str(mount_dir / "new")], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - check=True, + check=False, ) assert cp.stderr == b"" diff --git a/fsspec/tests/test_spec.py b/fsspec/tests/test_spec.py index 2f5ed73f1..6ab03abe5 100644 --- a/fsspec/tests/test_spec.py +++ b/fsspec/tests/test_spec.py @@ -1147,7 +1147,7 @@ def test_posix_tests_bash_stat(path, expected, glob_files_folder): f"cd {glob_files_folder} && shopt -s globstar && stat -c %N {bash_path}", ], capture_output=True, - check=True, + check=False, ) # Remove the last element always empty bash_output = bash_output.stdout.decode("utf-8").replace("'", "").split("\n")[:-1] From d0ceefb27bf8e1430b091e0fd3801b1704038eed Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 17 Jan 2024 07:17:08 -0800 Subject: [PATCH 6/7] Add pyproject requires_version --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index b556c2657..4458d6b6c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,6 @@ +[project] +requires-python = ">=3.8" + [tool.black] target_version = ['py310'] line-length = 88 From 248b9689c7d0cbcedb3713465c43798edcb8d687 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 17 Jan 2024 07:20:33 -0800 Subject: [PATCH 7/7] Revert "Add pyproject requires_version" This reverts commit d0ceefb27bf8e1430b091e0fd3801b1704038eed. --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4458d6b6c..b556c2657 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,3 @@ -[project] -requires-python = ">=3.8" - [tool.black] target_version = ['py310'] line-length = 88