From 7cec1ef1fc426790a1eb5d154badadbae27bf881 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 23 Aug 2024 17:42:07 +0900 Subject: [PATCH] test: enable pyright:reportUnnecessaryTypeIgnoreComment --- ops/__init__.py | 4 ++-- ops/_private/yaml.py | 2 +- ops/charm.py | 2 +- ops/framework.py | 16 ++++++++-------- ops/main.py | 2 +- ops/model.py | 10 +++++----- ops/pebble.py | 6 +++--- ops/storage.py | 4 ++-- ops/testing.py | 2 +- pyproject.toml | 1 + test/test_framework.py | 18 +++++++++--------- test/test_helpers.py | 4 ++-- test/test_jujuversion.py | 2 +- test/test_model.py | 14 +++++++------- test/test_pebble.py | 6 +++--- 15 files changed, 47 insertions(+), 46 deletions(-) diff --git a/ops/__init__.py b/ops/__init__.py index c4b0d950a..8fccdc619 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -175,7 +175,7 @@ # isort:skip_file # Import pebble explicitly. It's the one module we don't import names from below. -from . import pebble # type: ignore +from . import pebble # Also import charm explicitly. This is not strictly necessary as the # "from .charm" import automatically does that, but be explicit since this @@ -184,7 +184,7 @@ # Import the main module, which we've overriden in main.py to be callable. # This allows "import ops; ops.main(Charm)" to work as expected. -from . import main # type: ignore +from . import main # Explicitly import names from submodules so users can just "import ops" and # then use them as "ops.X". diff --git a/ops/_private/yaml.py b/ops/_private/yaml.py index 1ae0f8826..f3ceadb27 100644 --- a/ops/_private/yaml.py +++ b/ops/_private/yaml.py @@ -25,7 +25,7 @@ def safe_load(stream: Union[str, TextIO]) -> Any: """Same as yaml.safe_load, but use fast C loader if available.""" - return yaml.load(stream, Loader=_safe_loader) # type: ignore # noqa: S506 + return yaml.load(stream, Loader=_safe_loader) # noqa: S506 def safe_dump(data: Any, stream: Optional[TextIO] = None) -> str: diff --git a/ops/charm.py b/ops/charm.py index 5862b3947..b1550c824 100644 --- a/ops/charm.py +++ b/ops/charm.py @@ -1650,7 +1650,7 @@ def __init__(self, role: RelationRole, relation_name: str, raw: '_RelationMetaDi self.interface_name = raw['interface'] self.limit = limit = raw.get('limit', None) - if limit is not None and not isinstance(limit, int): # type: ignore + if limit is not None and not isinstance(limit, int): raise TypeError(f'limit should be an int, not {type(limit)}') self.scope = raw.get('scope') or self._default_scope diff --git a/ops/framework.py b/ops/framework.py index c47e6b9a0..11c09e50c 100644 --- a/ops/framework.py +++ b/ops/framework.py @@ -789,7 +789,7 @@ class SomeObject: event_kind = bound_event.event_kind emitter = bound_event.emitter - self.register_type(event_type, emitter, event_kind) # type: ignore + self.register_type(event_type, emitter, event_kind) if hasattr(emitter, 'handle'): emitter_path = emitter.handle.path @@ -824,7 +824,7 @@ def _next_event_key(self) -> str: """Return the next event key that should be used, incrementing the internal counter.""" # Increment the count first; this means the keys will start at 1, and 0 # means no events have been emitted. - self._stored['event_count'] += 1 # type: ignore #(we know event_count holds an int) + self._stored['event_count'] += 1 return str(self._stored['event_count']) def _emit(self, event: EventBase): @@ -956,7 +956,7 @@ def _reemit(self, single_event_path: Optional[str] = None): self._storage.drop_notice(event_path, observer_path, method_name) # We intentionally consider this event to be dead and reload it from # scratch in the next path. - self.framework._forget(event) # type: ignore + self.framework._forget(event) if not deferred and last_event_path is not None: self._storage.drop_snapshot(last_event_path) @@ -1068,10 +1068,10 @@ class BoundStoredState: if TYPE_CHECKING: # to help the type checker and IDEs: @property - def _data(self) -> StoredStateData: ... # type: ignore + def _data(self) -> StoredStateData: ... @property - def _attr_name(self) -> str: ... # type: ignore + def _attr_name(self) -> str: ... def __init__(self, parent: Object, attr_name: str): parent.framework.register_type(StoredStateData, parent) @@ -1086,7 +1086,7 @@ def __init__(self, parent: Object, attr_name: str): self.__dict__['_data'] = data self.__dict__['_attr_name'] = attr_name - parent.framework.observe(parent.framework.on.commit, self._data.on_commit) # type: ignore + parent.framework.observe(parent.framework.on.commit, self._data.on_commit) @typing.overload def __getattr__(self, key: Literal['on']) -> ObjectEvents: @@ -1228,14 +1228,14 @@ def _wrap_stored(parent_data: StoredStateData, value: Any) -> Any: def _unwrap_stored(parent_data: StoredStateData, value: Any) -> Any: if isinstance(value, (StoredDict, StoredList, StoredSet)): - return value._under # pyright: ignore[reportPrivateUsage] + return value._under return value def _wrapped_repr(obj: '_StoredObject') -> str: t = type(obj) if obj._under: - return f'{t.__module__}.{t.__name__}({obj._under!r})' # type: ignore + return f'{t.__module__}.{t.__name__}({obj._under!r})' else: return f'{t.__module__}.{t.__name__}()' diff --git a/ops/main.py b/ops/main.py index 871a749aa..d232a7fd2 100644 --- a/ops/main.py +++ b/ops/main.py @@ -437,7 +437,7 @@ def _setup_root_logging(self): self._model_backend, debug=self._juju_context.debug, exc_stderr=handling_action ) - logger.debug('ops %s up and running.', ops.__version__) # type:ignore + logger.debug('ops %s up and running.', ops.__version__) def _make_storage(self, dispatcher: _Dispatcher): charm_state_path = self._charm_root / self._charm_state_path diff --git a/ops/model.py b/ops/model.py index 434c0e23a..9102f30d0 100644 --- a/ops/model.py +++ b/ops/model.py @@ -896,7 +896,7 @@ def __getitem__(self, relation_name: str) -> List['Relation']: is_peer = relation_name in self._peers relation_list: Optional[List[Relation]] = self._data[relation_name] if not isinstance(relation_list, list): - relation_list = self._data[relation_name] = [] # type: ignore + relation_list = self._data[relation_name] = [] for rid in self._backend.relation_ids(relation_name): if rid == self._broken_relation_id: continue @@ -2083,7 +2083,7 @@ def __init__(self, storage_names: Iterable[str], backend: '_ModelBackend'): storage_name: None for storage_name in storage_names } - def __contains__(self, key: str): # pyright: ignore[reportIncompatibleMethodOverride] + def __contains__(self, key: str): return key in self._storage_map def __len__(self): @@ -2101,7 +2101,7 @@ def __getitem__(self, storage_name: str) -> List['Storage']: storage_list = self._storage_map[storage_name] = [] for storage_index in self._backend.storage_list(storage_name): storage = Storage(storage_name, storage_index, self._backend) - storage_list.append(storage) # type: ignore + storage_list.append(storage) return storage_list def request(self, storage_name: str, count: int = 1): @@ -3144,7 +3144,7 @@ def _format_action_result_dict( f"duplicate key detected in dictionary passed to 'action-set': {key!r}" ) else: - output_[key] = value # type: ignore + output_[key] = value return output_ @@ -3761,7 +3761,7 @@ def validate_metric_label(cls, label_name: str): @classmethod def format_metric_value(cls, value: Union[int, float]): - if not isinstance(value, (int, float)): # pyright: ignore[reportUnnecessaryIsInstance] + if not isinstance(value, (int, float)): raise ModelError( f'invalid metric value {value!r} provided:' ' must be a positive finite float' ) diff --git a/ops/pebble.py b/ops/pebble.py index 2b36b35cf..701b60db5 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -67,7 +67,7 @@ Union, ) -import websocket # type: ignore +import websocket from ops._private import timeconv, yaml @@ -1824,7 +1824,7 @@ def _reader_to_websocket( chunk = chunk.encode(encoding) ws.send_binary(chunk) - ws.send('{"command":"end"}') # type: ignore # Send "end" command as TEXT frame to signal EOF + ws.send('{"command":"end"}') # Send "end" command as TEXT frame to signal EOF def _websocket_to_writer(ws: _WebSocket, writer: _WebsocketWriter, encoding: Optional[str]): @@ -2891,7 +2891,7 @@ def exec( stdio_ws = self._connect_websocket(task_id, 'stdio') if not combine_stderr: stderr_ws = self._connect_websocket(task_id, 'stderr') - except websocket.WebSocketException as e: # type: ignore + except websocket.WebSocketException as e: # Error connecting to websockets, probably due to the exec/change # finishing early with an error. Call wait_change to pick that up. change = self.wait_change(ChangeID(change_id)) diff --git a/ops/storage.py b/ops/storage.py index 5024e5331..f9ba0fcc9 100644 --- a/ops/storage.py +++ b/ops/storage.py @@ -25,7 +25,7 @@ from pathlib import Path from typing import Any, Callable, Generator, List, Optional, Tuple, Union, cast -import yaml # pyright: ignore[reportMissingModuleSource] +import yaml logger = logging.getLogger() @@ -402,7 +402,7 @@ def get(self, key: str) -> Any: p = _run(['state-get', key], stdout=subprocess.PIPE, check=True) if p.stdout == '' or p.stdout == '\n': raise KeyError(key) - return yaml.load(p.stdout, Loader=_SimpleLoader) # type: ignore # noqa: S506 + return yaml.load(p.stdout, Loader=_SimpleLoader) # noqa: S506 def delete(self, key: str) -> None: """Remove a key from being tracked. diff --git a/ops/testing.py b/ops/testing.py index 02ab2eead..40d60fc4f 100644 --- a/ops/testing.py +++ b/ops/testing.py @@ -389,7 +389,7 @@ class TestEvents(self._charm_cls.on.__class__): TestEvents.__name__ = self._charm_cls.on.__class__.__name__ - class TestCharm(self._charm_cls): # type: ignore + class TestCharm(self._charm_cls): on = TestEvents() # Note: jam 2020-03-01 This is so that errors in testing say MyCharm has no attribute foo, diff --git a/pyproject.toml b/pyproject.toml index 967200095..58a7789e0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -215,5 +215,6 @@ reportMissingModuleSource = false reportPrivateUsage = false reportUnnecessaryIsInstance = false reportUnnecessaryComparison = false +reportUnnecessaryTypeIgnoreComment = "error" disableBytesTypePromotions = false stubPath = "" diff --git a/test/test_framework.py b/test/test_framework.py index d96a28656..ad92aef0f 100644 --- a/test/test_framework.py +++ b/test/test_framework.py @@ -354,8 +354,8 @@ def on_commit(self, event: ops.CommitEvent): framework.commit() - assert obs._stored.myinitdata == 41 # type: ignore - assert obs._stored.mydata == 42 # type: ignore + assert obs._stored.myinitdata == 41 + assert obs._stored.mydata == 42 assert obs.seen, [ops.PreCommitEvent, ops.CommitEvent] framework.close() @@ -363,11 +363,11 @@ def on_commit(self, event: ops.CommitEvent): new_obs = PreCommitObserver(other_framework, None) - assert obs._stored.myinitdata == 41 # type: ignore - assert new_obs._stored.mydata == 42 # type: ignore + assert obs._stored.myinitdata == 41 + assert new_obs._stored.mydata == 42 with pytest.raises(AttributeError): - new_obs._stored.myotherdata # type: ignore + new_obs._stored.myotherdata def test_defer_and_reemit(self, request: pytest.FixtureRequest): framework = create_framework(request) @@ -1083,7 +1083,7 @@ def test_stored_list_repr(self): ops.StoredList(None, [1, 2, 3]) # type: ignore ) == 'ops.framework.StoredList([1, 2, 3])' - ) # type: ignore + ) def test_stored_set_repr(self): assert repr(ops.StoredSet(None, set())) == 'ops.framework.StoredSet()' # type: ignore @@ -1159,14 +1159,14 @@ class _StoredProtocol(typing.Protocol): assert isinstance(obj, _StoredProtocol) try: - obj._stored.foo # type: ignore + obj._stored.foo except AttributeError as e: assert str(e) == "attribute 'foo' is not stored" else: pytest.fail('AttributeError not raised') try: - obj._stored.on = 'nonono' # type: ignore + obj._stored.on = 'nonono' except AttributeError as e: assert str(e) == "attribute 'on' is reserved and cannot be set" else: @@ -1850,7 +1850,7 @@ def test_breakpoint_reserved_names(self, request: pytest.FixtureRequest, name: s def test_breakpoint_not_really_names(self, request: pytest.FixtureRequest, name: typing.Any): framework = create_framework(request) with pytest.raises(TypeError) as excinfo: - framework.breakpoint(name) # type: ignore + framework.breakpoint(name) assert str(excinfo.value) == 'breakpoint names must be strings' def check_trace_set( diff --git a/test/test_helpers.py b/test/test_helpers.py index 2bd2a9df8..a97fa8e06 100644 --- a/test/test_helpers.py +++ b/test/test_helpers.py @@ -105,7 +105,7 @@ class TestCharmEvents(ops.CharmEvents): if meta is None: meta = ops.CharmMeta() - model = ops.Model(meta, _ModelBackend('local/0')) # type: ignore + model = ops.Model(meta, _ModelBackend('local/0')) # We can pass foo_event as event_name because we're not actually testing dispatch. framework = ops.Framework( SQLiteStorage(':memory:'), @@ -175,7 +175,7 @@ def write(self, name: str, content: str): path.chmod(0o755) # TODO: this hardcodes the path to bash.exe, which works for now but might # need to be set via environ or something like that. - path.with_suffix('.bat').write_text( # type: ignore + path.with_suffix('.bat').write_text( f'@"C:\\Program Files\\git\\bin\\bash.exe" {path} %*\n' ) diff --git a/test/test_jujuversion.py b/test/test_jujuversion.py index 07d42079b..713248bfa 100644 --- a/test/test_jujuversion.py +++ b/test/test_jujuversion.py @@ -43,7 +43,7 @@ def test_parsing(vs: str, major: int, minor: int, tag: str, patch: int, build: i assert v.build == build -@unittest.mock.patch('os.environ', new={}) # type: ignore +@unittest.mock.patch('os.environ', new={}) def test_from_environ(): # JUJU_VERSION is not set v = ops.JujuVersion.from_environ() diff --git a/test/test_model.py b/test/test_model.py index b72b9972c..c8266dc90 100644 --- a/test/test_model.py +++ b/test/test_model.py @@ -1865,7 +1865,7 @@ def test_get_checks(self, container: ops.Container): 'status': 'up', 'failures': 0, 'threshold': 3, - }), # type: ignore + }), pebble.CheckInfo.from_dict({ 'name': 'c2', 'level': 'alive', @@ -1911,7 +1911,7 @@ def test_get_check(self, container: ops.Container): 'status': 'up', 'failures': 0, 'threshold': 3, - }) # type: ignore + }) ]) c = container.get_check('c1') assert container.pebble.requests == [('get_checks', None, ('c1',))] # type: ignore @@ -1934,7 +1934,7 @@ def test_get_check(self, container: ops.Container): 'status': 'up', 'failures': 0, 'threshold': 3, - }), # type: ignore + }), pebble.CheckInfo.from_dict({ 'name': 'c2', 'level': 'alive', @@ -3206,7 +3206,7 @@ def test_invalid_metric_values(self): ({'a': float('-inf')}, {}), ({'a': float('nan')}, {}), ({'foo': 'bar'}, {}), # type: ignore - ({'foo': '1O'}, {}), # type: ignore + ({'foo': '1O'}, {}), ] for metrics, labels in invalid_inputs: with pytest.raises(ops.ModelError): @@ -3454,7 +3454,7 @@ def test_unit_add_secret_errors(self, model: ops.Model): ] for content, kwargs, exc_type in errors: with pytest.raises(exc_type): - model.unit.add_secret(content, **kwargs) # type: ignore + model.unit.add_secret(content, **kwargs) def test_add_secret_errors(self, model: ops.Model): errors: typing.Any = [ @@ -3474,9 +3474,9 @@ def test_add_secret_errors(self, model: ops.Model): ] for content, kwargs, exc_type in errors: with pytest.raises(exc_type): - model.app.add_secret(content, **kwargs) # type: ignore + model.app.add_secret(content, **kwargs) with pytest.raises(exc_type): - model.unit.add_secret(content, **kwargs) # type: ignore + model.unit.add_secret(content, **kwargs) def test_get_secret_id(self, fake_script: FakeScript, model: ops.Model): fake_script.write('secret-get', """echo '{"foo": "g"}'""") diff --git a/test/test_pebble.py b/test/test_pebble.py index 01f07806e..b5e723fcb 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -26,7 +26,7 @@ import unittest.util import pytest -import websocket # type: ignore +import websocket import test.fake_pebble as fake_pebble from ops import pebble @@ -3338,8 +3338,8 @@ def test_wait_exit_nonzero(self, client: MockClient): process.wait() assert excinfo.value.command == ['false'] assert excinfo.value.exit_code == 1 - assert excinfo.value.stdout is None # type: ignore - assert excinfo.value.stderr is None # type: ignore + assert excinfo.value.stdout is None + assert excinfo.value.stderr is None assert client.requests == [ ('POST', '/v1/exec', None, self.build_exec_data(['false'])),