test: enable pyright:reportUnnecessaryTypeIgnoreComment

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".

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:

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

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__}()'
 

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

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
@@ -2079,7 +2079,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):
@@ -2097,7 +2097,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):
@@ -3140,7 +3140,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_
 
@@ -3757,7 +3757,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'
             )

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))

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.

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,

pyproject.toml

@@ -215,5 +215,6 @@ reportMissingModuleSource = false
 reportPrivateUsage = false
 reportUnnecessaryIsInstance = false
 reportUnnecessaryComparison = false
+reportUnnecessaryTypeIgnoreComment = "error"
 disableBytesTypePromotions = false
 stubPath = ""

test/test_framework.py

@@ -354,20 +354,20 @@ 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()
 
         other_framework = create_framework(request, tmpdir=tmp_path)
 
         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(

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'
         )
 

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()

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"}'""")

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'])),