From 66bd8fdbcdbc3329694614870d1930718cf5baa7 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 21 Jan 2025 19:12:09 +0000 Subject: [PATCH 1/5] Enable strict type checking with mypy - Update mypy configuration with stricter type checking rules - Add more type stubs to pre-commit configuration - Run mypy both through pre-commit and directly in CI - Install project in editable mode for better type checking - Set correct PYTHONPATH in CI environment --- .github/workflows/lint.yml | 12 ++++++++++ dev_config/python/.pre-commit-config.yaml | 26 +++++++++++++++++++-- dev_config/python/mypy.ini | 28 +++++++++++++++++++---- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 789a938e1d7e..170ea3560e27 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,8 +51,20 @@ jobs: cache: 'pip' - name: Install pre-commit run: pip install pre-commit==3.7.0 + - name: Install project in editable mode + run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml + - name: Run mypy directly (for better error reporting) + if: always() # Run even if pre-commit fails + run: | + pip install mypy==1.9.0 \ + types-requests types-setuptools types-pyyaml types-toml \ + types-redis types-protobuf types-python-dateutil types-pyjwt \ + types-docutils types-Pillow types-boto3 types-cryptography \ + types-Jinja2 types-MarkupSafe types-click types-filelock \ + types-decorator types-docopt types-emoji types-enum34 types-paramiko + PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: diff --git a/dev_config/python/.pre-commit-config.yaml b/dev_config/python/.pre-commit-config.yaml index 0ae868fe70fd..25569a132af1 100644 --- a/dev_config/python/.pre-commit-config.yaml +++ b/dev_config/python/.pre-commit-config.yaml @@ -36,8 +36,30 @@ repos: rev: v1.9.0 hooks: - id: mypy + name: mypy (with strict type checking) additional_dependencies: - [types-requests, types-setuptools, types-pyyaml, types-toml] - entry: mypy --config-file dev_config/python/mypy.ini openhands/ + - types-requests + - types-setuptools + - types-pyyaml + - types-toml + - types-redis + - types-protobuf + - types-python-dateutil + - types-pyjwt + - types-docutils + - types-Pillow + - types-boto3 + - types-cryptography + - types-Jinja2 + - types-MarkupSafe + - types-click + - types-filelock + - types-decorator + - types-docopt + - types-emoji + - types-enum34 + - types-paramiko + entry: mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ always_run: true pass_filenames: false + verbose: true diff --git a/dev_config/python/mypy.ini b/dev_config/python/mypy.ini index 84b97d720b2f..72081060a20d 100644 --- a/dev_config/python/mypy.ini +++ b/dev_config/python/mypy.ini @@ -1,9 +1,29 @@ [mypy] -warn_unused_configs = True +# Error output +show_error_codes = True +pretty = True +show_column_numbers = True + +# Import discovery ignore_missing_imports = True +follow_imports = normal + +# Untyped definitions and calls +disallow_untyped_defs = True check_untyped_defs = True -explicit_package_bases = True -warn_unreachable = True -warn_redundant_casts = True +disallow_incomplete_defs = True +disallow_untyped_decorators = True + +# None and Optional handling no_implicit_optional = True strict_optional = True + +# Warnings +warn_unused_configs = True +warn_redundant_casts = True +warn_unused_ignores = True +warn_return_any = True +warn_unreachable = True + +# Misc +explicit_package_bases = True From 7a259915c1748b1b9e999f5f02450658e95e4c7e Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Tue, 21 Jan 2025 14:47:51 -0500 Subject: [PATCH 2/5] Update .github/workflows/lint.yml --- .github/workflows/lint.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 170ea3560e27..db7ae1f9c90f 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -55,16 +55,6 @@ jobs: run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml - - name: Run mypy directly (for better error reporting) - if: always() # Run even if pre-commit fails - run: | - pip install mypy==1.9.0 \ - types-requests types-setuptools types-pyyaml types-toml \ - types-redis types-protobuf types-python-dateutil types-pyjwt \ - types-docutils types-Pillow types-boto3 types-cryptography \ - types-Jinja2 types-MarkupSafe types-click types-filelock \ - types-decorator types-docopt types-emoji types-enum34 types-paramiko - PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: From 64ebef3646487a92ca2acd639a262620f9c67ac5 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Tue, 21 Jan 2025 14:52:56 -0500 Subject: [PATCH 3/5] Update .github/workflows/lint.yml --- .github/workflows/lint.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index db7ae1f9c90f..170ea3560e27 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -55,6 +55,16 @@ jobs: run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml + - name: Run mypy directly (for better error reporting) + if: always() # Run even if pre-commit fails + run: | + pip install mypy==1.9.0 \ + types-requests types-setuptools types-pyyaml types-toml \ + types-redis types-protobuf types-python-dateutil types-pyjwt \ + types-docutils types-Pillow types-boto3 types-cryptography \ + types-Jinja2 types-MarkupSafe types-click types-filelock \ + types-decorator types-docopt types-emoji types-enum34 types-paramiko + PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: From 9d5d39c17e54871cc09f46fedcfd6aa42a69abc9 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 19 Feb 2025 01:45:56 +0000 Subject: [PATCH 4/5] Fix mypy errors in storage directory --- openhands/storage/conversation/conversation_store.py | 2 +- openhands/storage/conversation/file_conversation_store.py | 2 +- openhands/storage/google_cloud.py | 2 +- openhands/storage/local.py | 2 +- openhands/storage/s3.py | 2 +- openhands/storage/settings/file_settings_store.py | 2 +- openhands/storage/settings/settings_store.py | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index 8c55920efa62..efe95b1b6983 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -15,7 +15,7 @@ class ConversationStore(ABC): """ @abstractmethod - async def save_metadata(self, metadata: ConversationMetadata): + async def save_metadata(self, metadata: ConversationMetadata) -> None: """Store conversation metadata""" @abstractmethod diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index 79e04921078e..0f7d1f9fa961 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -29,7 +29,7 @@ class FileConversationStore(ConversationStore): file_store: FileStore - async def save_metadata(self, metadata: ConversationMetadata): + async def save_metadata(self, metadata: ConversationMetadata) -> None: json_str = conversation_metadata_type_adapter.dump_json(metadata) path = self.get_conversation_metadata_filename(metadata.conversation_id) await call_sync_from_async(self.file_store.write, path, json_str) diff --git a/openhands/storage/google_cloud.py b/openhands/storage/google_cloud.py index 19465c0442ce..24466e81ef8a 100644 --- a/openhands/storage/google_cloud.py +++ b/openhands/storage/google_cloud.py @@ -29,7 +29,7 @@ def read(self, path: str) -> str: blob = self.bucket.blob(path) try: with blob.open('r') as f: - return f.read() + return str(f.read()) except NotFound as err: raise FileNotFoundError(err) diff --git a/openhands/storage/local.py b/openhands/storage/local.py index 38815bce1f0d..711a364cf5d1 100644 --- a/openhands/storage/local.py +++ b/openhands/storage/local.py @@ -17,7 +17,7 @@ def get_full_path(self, path: str) -> str: path = path[1:] return os.path.join(self.root, path) - def write(self, path: str, contents: str | bytes): + def write(self, path: str, contents: str | bytes) -> None: full_path = self.get_full_path(path) os.makedirs(os.path.dirname(full_path), exist_ok=True) mode = 'w' if isinstance(contents, str) else 'wb' diff --git a/openhands/storage/s3.py b/openhands/storage/s3.py index cb723355155d..4fb59b920d6c 100644 --- a/openhands/storage/s3.py +++ b/openhands/storage/s3.py @@ -46,7 +46,7 @@ def read(self, path: str) -> str: try: response = self.client.get_object(Bucket=self.bucket, Key=path) with response['Body'] as stream: - return stream.read().decode('utf-8') + return str(stream.read().decode('utf-8')) except botocore.exceptions.ClientError as e: # Catch all S3-related errors if e.response['Error']['Code'] == 'NoSuchBucket': diff --git a/openhands/storage/settings/file_settings_store.py b/openhands/storage/settings/file_settings_store.py index d3cc08677078..3c5cf9584891 100644 --- a/openhands/storage/settings/file_settings_store.py +++ b/openhands/storage/settings/file_settings_store.py @@ -25,7 +25,7 @@ async def load(self) -> Settings | None: except FileNotFoundError: return None - async def store(self, settings: Settings): + async def store(self, settings: Settings) -> None: json_str = settings.model_dump_json(context={'expose_secrets': True}) await call_sync_from_async(self.file_store.write, self.path, json_str) diff --git a/openhands/storage/settings/settings_store.py b/openhands/storage/settings/settings_store.py index cd64ae8e1ea5..c9fdd093ccb9 100644 --- a/openhands/storage/settings/settings_store.py +++ b/openhands/storage/settings/settings_store.py @@ -16,7 +16,7 @@ async def load(self) -> Settings | None: """Load session init data""" @abstractmethod - async def store(self, settings: Settings): + async def store(self, settings: Settings) -> None: """Store session init data""" @classmethod From 0ce73310782c65af02575f599f9fcd00cf5045bb Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 19 Feb 2025 01:56:15 +0000 Subject: [PATCH 5/5] Revert all files outside of openhands/storage/ back to main --- .github/workflows/lint.yml | 12 ---------- dev_config/python/.pre-commit-config.yaml | 26 ++------------------- dev_config/python/mypy.ini | 28 ++++------------------- 3 files changed, 6 insertions(+), 60 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 170ea3560e27..789a938e1d7e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,20 +51,8 @@ jobs: cache: 'pip' - name: Install pre-commit run: pip install pre-commit==3.7.0 - - name: Install project in editable mode - run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml - - name: Run mypy directly (for better error reporting) - if: always() # Run even if pre-commit fails - run: | - pip install mypy==1.9.0 \ - types-requests types-setuptools types-pyyaml types-toml \ - types-redis types-protobuf types-python-dateutil types-pyjwt \ - types-docutils types-Pillow types-boto3 types-cryptography \ - types-Jinja2 types-MarkupSafe types-click types-filelock \ - types-decorator types-docopt types-emoji types-enum34 types-paramiko - PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: diff --git a/dev_config/python/.pre-commit-config.yaml b/dev_config/python/.pre-commit-config.yaml index 25569a132af1..0ae868fe70fd 100644 --- a/dev_config/python/.pre-commit-config.yaml +++ b/dev_config/python/.pre-commit-config.yaml @@ -36,30 +36,8 @@ repos: rev: v1.9.0 hooks: - id: mypy - name: mypy (with strict type checking) additional_dependencies: - - types-requests - - types-setuptools - - types-pyyaml - - types-toml - - types-redis - - types-protobuf - - types-python-dateutil - - types-pyjwt - - types-docutils - - types-Pillow - - types-boto3 - - types-cryptography - - types-Jinja2 - - types-MarkupSafe - - types-click - - types-filelock - - types-decorator - - types-docopt - - types-emoji - - types-enum34 - - types-paramiko - entry: mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ + [types-requests, types-setuptools, types-pyyaml, types-toml] + entry: mypy --config-file dev_config/python/mypy.ini openhands/ always_run: true pass_filenames: false - verbose: true diff --git a/dev_config/python/mypy.ini b/dev_config/python/mypy.ini index 72081060a20d..84b97d720b2f 100644 --- a/dev_config/python/mypy.ini +++ b/dev_config/python/mypy.ini @@ -1,29 +1,9 @@ [mypy] -# Error output -show_error_codes = True -pretty = True -show_column_numbers = True - -# Import discovery +warn_unused_configs = True ignore_missing_imports = True -follow_imports = normal - -# Untyped definitions and calls -disallow_untyped_defs = True check_untyped_defs = True -disallow_incomplete_defs = True -disallow_untyped_decorators = True - -# None and Optional handling +explicit_package_bases = True +warn_unreachable = True +warn_redundant_casts = True no_implicit_optional = True strict_optional = True - -# Warnings -warn_unused_configs = True -warn_redundant_casts = True -warn_unused_ignores = True -warn_return_any = True -warn_unreachable = True - -# Misc -explicit_package_bases = True