Add Python formatting checks

Author: Kimahriman

.github/workflows/python-test.yml

@@ -13,21 +13,6 @@ defaults:
     working-directory: ./python
 
 jobs:
-  lint:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-      - name: Setup Python
-        uses: actions/setup-python@v4
-        with:
-          python-version: 3.12
-      - name: Install dev dependencies
-        run: |
-          pip3 install -r requirements-dev.txt
-      - name: Type checking
-        run: |
-          mypy python
-
   test:
     strategy:
       fail-fast: false
@@ -68,13 +53,19 @@ jobs:
           sccache: 'true'
           container: 'off'
           working-directory: ./python
-          args: --features kerberos
+          args: --features kerberos --extras devel
 
-      - name: Install dev dependencies and run tests
+      - name: Run lints
+        run: |
+          source .venv/bin/activate
+          mypy
+          isort . --check
+          black . --check
+
+      - name: Run tests
         run: |
           source .venv/bin/activate
-          pip3 install -r requirements-dev.txt
-          pytest
+          mypy
 
   build:
     runs-on: ubuntu-latest

python/README.md

@@ -22,7 +22,6 @@ The same requirements apply as the Rust tests, requiring Java, Maven, Hadoop, an
 python3 -m venv .venv
 source .venv/bin/activate
 pip3 install maturin
-pip3 install -r requirements-dev.txt
-maturin develop
+maturin develop -E devel
 pytest
 ```

python/conftest.py

@@ -1,12 +1,14 @@
+import subprocess
+import urllib
 import urllib.parse
+
 import fsspec
-import subprocess
 import pytest
-import urllib
 
 from hdfs_native.fsspec import HdfsFileSystem
 
-@pytest.fixture(scope='module')
+
+@pytest.fixture(scope="module")
 def minidfs():
     child = subprocess.Popen(
         [
@@ -23,7 +25,7 @@ def minidfs():
         stderr=subprocess.DEVNULL,
         universal_newlines=True,
         encoding="utf8",
-        bufsize=0
+        bufsize=0,
     )
 
     output = child.stdout.readline().strip()
@@ -36,7 +38,8 @@ def minidfs():
     except:
         child.kill()
 
-@pytest.fixture(scope='module')
+
+@pytest.fixture(scope="module")
 def fs(minidfs: str) -> HdfsFileSystem:
     url = urllib.parse.urlparse(minidfs)
-    return fsspec.filesystem(url.scheme, host=url.hostname, port=url.port)
+    return fsspec.filesystem(url.scheme, host=url.hostname, port=url.port)

python/hdfs_native/__init__.py

@@ -1,22 +1,25 @@
 import io
 import os
 from typing import Dict, Iterator, Optional
-from typing_extensions import Buffer
+
+# For some reason mypy doesn't think this exists
+from typing_extensions import Buffer  # type: ignore
 
 from ._internal import *
 
+
 class FileReader(io.RawIOBase):
-    
+
     def __init__(self, inner: "RawFileReader"):
         self.inner = inner
 
     def __len__(self) -> int:
         return self.inner.file_length()
-    
+
     def __enter__(self):
         # Don't need to do anything special here
         return self
-    
+
     def __exit__(self, *_args):
         # Future updates could close the file manually here if that would help clean things up
         pass
@@ -34,11 +37,11 @@ def seek(self, offset: int, whence=os.SEEK_SET):
         elif whence == os.SEEK_END:
             self.inner.seek(self.inner.file_length() + offset)
         else:
-            raise ValueError(f'Unsupported whence {whence}')
+            raise ValueError(f"Unsupported whence {whence}")
 
     def seekable(self):
         return True
-    
+
     def tell(self) -> int:
         return self.inner.tell()
 
@@ -48,17 +51,18 @@ def readable(self) -> bool:
     def read(self, size: int = -1) -> bytes:
         """Read up to `size` bytes from the file, or all content if -1"""
         return self.inner.read(size)
-    
+
     def readall(self) -> bytes:
         return self.read()
 
     def read_range(self, offset: int, len: int) -> bytes:
         """Read `len` bytes from the file starting at `offset`. Doesn't affect the position in the file"""
         return self.inner.read_range(offset, len)
-    
+
     def close(self) -> None:
         pass
 
+
 class FileWriter(io.RawIOBase):
 
     def __init__(self, inner: "RawFileWriter"):
@@ -77,10 +81,11 @@ def close(self) -> None:
 
     def __enter__(self) -> "FileWriter":
         return self
-    
+
     def __exit__(self, *_args):
         self.close()
 
+
 class Client:
 
     def __init__(self, url: str, config: Optional[Dict[str, str]] = None):
@@ -101,7 +106,7 @@ def read(self, path: str) -> FileReader:
     def create(self, path: str, write_options: WriteOptions) -> FileWriter:
         """Creates a new file and opens it for writing at `path`"""
         return FileWriter(self.inner.create(path, write_options))
-    
+
     def append(self, path: str) -> FileWriter:
         """Opens an existing file to append to at `path`"""
         return FileWriter(self.inner.append(path))
@@ -129,15 +134,20 @@ def delete(self, path: str, recursive: bool) -> bool:
         is a non-empty directory, this will fail.
         """
         return self.inner.delete(path, recursive)
-    
+
     def set_times(self, path: str, mtime: int, atime: int) -> None:
         """
         Changes the modification time and access time of the file at `path` to `mtime` and `atime`, respectively.
         """
         return self.inner.set_times(path, mtime, atime)
 
-    def set_owner(self, path: str, owner: Optional[str] = None, group: Optional[str] = None) -> None:
+    def set_owner(
+        self,
+        path: str,
+        owner: Optional[str] = None,
+        group: Optional[str] = None,
+    ) -> None:
         """
         Sets the owner and/or group for the file at `path`
         """
-        return self.inner.set_owner(path, owner, group)
+        return self.inner.set_owner(path, owner, group)

python/hdfs_native/_internal.pyi

@@ -1,6 +1,7 @@
 from typing import Dict, Iterator, Optional
-from typing_extensions import Buffer
 
+# For some reason mypy doesn't think this exists
+from typing_extensions import Buffer  # type: ignore
 
 class FileStatus:
     path: str
@@ -44,23 +45,18 @@ class RawFileWriter:
 
 class RawClient:
     def __init__(self, url: str, config: Optional[Dict[str, str]]) -> None: ...
-
     def get_file_info(self, path: str) -> FileStatus: ...
-
     def list_status(self, path: str, recursive: bool) -> Iterator[FileStatus]: ...
-
     def read(self, path: str) -> RawFileReader: ...
-
     def create(self, path: str, write_options: WriteOptions) -> RawFileWriter: ...
-
     def append(self, path: str) -> RawFileWriter: ...
-
     def mkdirs(self, path: str, permission: int, create_parent: bool) -> None: ...
-
     def rename(self, src: str, dst: str, overwrite: bool) -> None: ...
-
     def delete(self, path: str, recursive: bool) -> bool: ...
-
     def set_times(self, path: str, mtime: int, atime: int) -> None: ...
-
-    def set_owner(self, path: str, owner: Optional[str], group: Optional[str]) -> None: ...
+    def set_owner(
+        self,
+        path: str,
+        owner: Optional[str],
+        group: Optional[str],
+    ) -> None: ...

python/hdfs_native/fsspec.py

@@ -1,9 +1,10 @@
-from contextlib import suppress
-from datetime import datetime
 import secrets
 import shutil
 import time
+from contextlib import suppress
+from datetime import datetime
 from typing import TYPE_CHECKING, Dict, List, Optional, Union
+
 from fsspec import AbstractFileSystem
 from fsspec.utils import tokenize
 
@@ -12,36 +13,37 @@
 if TYPE_CHECKING:
     from . import FileStatus
 
+
 class HdfsFileSystem(AbstractFileSystem):
     def __init__(self, host: str, port: Optional[int] = None, *args, **storage_options):
         super().__init__(host, port, *args, **storage_options)
         self.host = host
         self.port = port
-        url = f'{self.protocol}://{host}'
+        url = f"{self.protocol}://{host}"
         if port:
-            url += f':{port}'
+            url += f":{port}"
         self.client = Client(url)
 
     @property
     def fsid(self):
-        return f'hdfs_native_{tokenize(self.protocol, self.host, self.port)}'
+        return f"hdfs_native_{tokenize(self.protocol, self.host, self.port)}"
 
-    def _convert_file_status(self, file_status: 'FileStatus') -> Dict:
+    def _convert_file_status(self, file_status: "FileStatus") -> Dict:
         return {
-            'name': file_status.path,
-            'size': file_status.length,
-            'type': 'directory' if file_status.isdir else 'file',
-            'permission': file_status.permission,
-            'owner': file_status.owner,
-            'group': file_status.group,
-            'modification_time': file_status.modification_time,
-            'access_time': file_status.access_time
+            "name": file_status.path,
+            "size": file_status.length,
+            "type": "directory" if file_status.isdir else "file",
+            "permission": file_status.permission,
+            "owner": file_status.owner,
+            "group": file_status.group,
+            "modification_time": file_status.modification_time,
+            "access_time": file_status.access_time,
         }
 
     def info(self, path, **_kwargs) -> Dict:
         file_status = self.client.get_file_info(path)
         return self._convert_file_status(file_status)
-    
+
     def exists(self, path, **_kwargs):
         try:
             self.info(path)
@@ -58,23 +60,23 @@ def ls(self, path: str, detail=True, **kwargs) -> List[Union[str, Dict]]:
 
     def touch(self, path: str, truncate=True, **kwargs):
         if truncate or not self.exists(path):
-            with self.open(path, 'wb', **kwargs):
+            with self.open(path, "wb", **kwargs):
                 pass
         else:
             now = int(time.time() * 1000)
             self.client.set_times(path, now, now)
 
     def mkdir(self, path: str, create_parents=True, **kwargs):
-        self.client.mkdirs(path, kwargs.get('permission', 0o755), create_parents)
+        self.client.mkdirs(path, kwargs.get("permission", 0o755), create_parents)
 
     def makedirs(self, path: str, exist_ok=False):
         if not exist_ok and self.exists(path):
-            raise FileExistsError('File or directory already exists')
+            raise FileExistsError("File or directory already exists")
 
         return self.mkdir(path, create_parents=True)
 
     def mv(self, path1: str, path2: str, **kwargs):
-        self.client.rename(path1, path2, kwargs.get('overwrite', False))
+        self.client.rename(path1, path2, kwargs.get("overwrite", False))
 
     def cp_file(self, path1, path2, **kwargs):
         with self._open(path1, "rb") as lstream:
@@ -93,7 +95,7 @@ def rmdir(self, path: str) -> None:
 
     def rm(self, path: str, recursive=False, maxdepth: Optional[int] = None) -> None:
         if maxdepth is not None:
-            raise NotImplementedError('maxdepth is not supported')
+            raise NotImplementedError("maxdepth is not supported")
         self.client.delete(path, recursive)
 
     def rm_file(self, path: str):
@@ -110,19 +112,19 @@ def _open(
         overwrite=True,
         replication: Optional[int] = None,
         block_size: Optional[int] = None,
-        **_kwargs
+        **_kwargs,
     ):
-        if mode == 'rb':
+        if mode == "rb":
             return self.client.read(path)
-        elif mode == 'wb':
+        elif mode == "wb":
             write_options = WriteOptions()
             write_options.overwrite = overwrite
             if replication:
                 write_options.replication = replication
             if block_size:
                 write_options.block_size = block_size
             return self.client.create(path, write_options=write_options)
-        elif mode == 'ab':
+        elif mode == "ab":
             return self.client.append(path)
         else:
-            raise ValueError(f'Mode {mode} is not supported')
+            raise ValueError(f"Mode {mode} is not supported")