ci: make Pyright report unnecessary type ignore comments

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

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