chore: remove as much 'if typing.TYPE_CHECKING:' as possible (#1034)

* Wherever possible, remove the 'if typing.TYPE_CHECKING:' guards.
* Move the definition of the `_ConfigOption` `TypedDict` from `ops.testing` to `ops.model` - `model` should not import from `testing` (the import loop was avoided because it was only when `TYPE_CHECKING` but it's still poor practice)

This leaves a few cases where we still use `TYPE_CHECKING`:
* When using `typing_extensions`. We currently assume that if `TYPE_CHECKING` is true that the `requirements-dev` have been installed (or at least `typing_extensions` has). It seems like this would not always be the case (e.g. if type checking is being done on a charm that uses `ops`) but is the status-quo and avoids bringing `typing_extensions` into `requirements.txt`. Hopefully, we won't need it for anything after `Required`/`NotRequired` and once we have progressed our minimum Python version enough to get those, we can drop it entirely.
* We override the definition of some class attributes to be properties when `TYPE_CHECKING` is true. Removing the guard would change the actual attributes.
* In Python 3.8 we can't use `weakref.WeakValueDictionary[T]` with a type. We're currently assuming that a more recent version of Python will be used when type checking, but can't bring this out as that would raise the minimum Python version for all cases (or we'd need to adjust the typing so that it can work in 3.8).

docs/conf.py

@@ -73,23 +73,25 @@ def _compute_navigation_tree(context):
 # domain name if present. Example entries would be ('py:func', 'int') or
 # ('envvar', 'LD_LIBRARY_PATH').
 nitpick_ignore = [
-    ('py:class', '_AddressDict'),
+    ('py:class', 'ops.model._AddressDict'),
     ('py:class', '_ChangeDict'),
     ('py:class', '_CheckInfoDict'),
+    ('py:class', 'ops.model._ConfigOption'),
+    ('py:class', 'ops.pebble._FileLikeIO'),
     ('py:class', '_FileInfoDict'),
-    ('py:class', '_IOSource'),
-    ('py:class', '_NetworkDict'),
+    ('py:class', 'ops.pebble._IOSource'),
+    ('py:class', 'ops.model._NetworkDict'),
     ('py:class', '_ProgressDict'),
     ('py:class', '_Readable'),
     ('py:class', '_RelationMetaDict'),
     ('py:class', '_ResourceMetaDict'),
-    ('py:class', '_ServiceInfoDict'),
+    ('py:class', 'ops.pebble._ServiceInfoDict'),
     ('py:class', '_StorageMetaDict'),
-    ('py:class', '_SystemInfoDict'),
+    ('py:class', 'ops.pebble._SystemInfoDict'),
     ('py:class', '_TaskDict'),
     ('py:class', '_TextOrBinaryIO'),
     ('py:class', '_WarningDict'),
-    ('py:class', '_WebSocket'),
+    ('py:class', 'ops.pebble._WebSocket'),
     ('py:class', '_Writeable'),
     ('py:class', 'ops.model._ModelBackend'),
     ('py:class', 'ops.model._ModelCache'),

ops/charm.py

@@ -35,14 +35,18 @@
 
 from ops import model
 from ops._private import yaml
-from ops.framework import EventBase, EventSource, Framework, Object, ObjectEvents
+from ops.framework import (
+    EventBase,
+    EventSource,
+    Framework,
+    Handle,
+    Object,
+    ObjectEvents,
+)
 
 if TYPE_CHECKING:
     from typing_extensions import Required
 
-    from ops.framework import Handle
-    from ops.model import Container, Relation, Storage
-
     _Scopes = Literal['global', 'container']
     _RelationMetaDict = TypedDict(
         '_RelationMetaDict', {
@@ -369,7 +373,7 @@ class RelationEvent(HookEvent):
     relations with the same name.
     """
 
-    relation: 'Relation'
+    relation: 'model.Relation'
     """The relation involved in this event."""
 
     # TODO(benhoyt): I *think* app should never be None, but confirm and update type
@@ -383,7 +387,7 @@ class RelationEvent(HookEvent):
     :class:`Application <model.Application>`-level event.
     """
 
-    def __init__(self, handle: 'Handle', relation: 'Relation',
+    def __init__(self, handle: 'Handle', relation: 'model.Relation',
                  app: Optional[model.Application] = None,
                  unit: Optional[model.Unit] = None):
         super().__init__(handle)
@@ -498,7 +502,7 @@ class RelationDepartedEvent(RelationEvent):
     relation, the unit agent will fire the :class:`RelationBrokenEvent`.
     """
 
-    def __init__(self, handle: 'Handle', relation: 'Relation',
+    def __init__(self, handle: 'Handle', relation: 'model.Relation',
                  app: Optional[model.Application] = None,
                  unit: Optional[model.Unit] = None,
                  departing_unit_name: Optional[str] = None):
@@ -563,10 +567,10 @@ class StorageEvent(HookEvent):
     of :class:`StorageEvent`.
     """
 
-    storage: 'Storage'
+    storage: 'model.Storage'
     """Storage instance this event refers to."""
 
-    def __init__(self, handle: 'Handle', storage: 'Storage'):
+    def __init__(self, handle: 'Handle', storage: 'model.Storage'):
         super().__init__(handle)
         self.storage = storage
 
@@ -645,15 +649,15 @@ class WorkloadEvent(HookEvent):
     a :class:`PebbleReadyEvent`.
     """
 
-    workload: 'Container'
+    workload: 'model.Container'
     """The workload involved in this event.
 
     Workload currently only can be a :class:`Container <model.Container>`, but
     in future may be other types that represent the specific workload type,
     for example a machine.
     """
 
-    def __init__(self, handle: 'Handle', workload: 'Container'):
+    def __init__(self, handle: 'Handle', workload: 'model.Container'):
         super().__init__(handle)
 
         self.workload = workload

ops/framework.py

@@ -37,7 +37,9 @@
     Hashable,
     Iterable,
     List,
+    Literal,
     Optional,
+    Protocol,
     Set,
     Tuple,
     Type,
@@ -46,6 +48,7 @@
 )
 
 from ops import charm
+from ops.model import Model, _ModelBackend
 from ops.storage import JujuStorage, NoSnapshotError, SQLiteStorage
 
 
@@ -62,23 +65,18 @@ def snapshot(self) -> Dict[str, Any]: ...  # noqa
     def restore(self, snapshot: Dict[str, Any]) -> None: ...  # noqa
 
 
-if TYPE_CHECKING:
-    from typing import Literal, Protocol
+class _StoredObject(Protocol):
+    _under: Any = None  # noqa
 
-    from ops.charm import CharmMeta
-    from ops.model import Model, _ModelBackend
 
-    class _StoredObject(Protocol):
-        _under: Any = None  # noqa
+StoredObject = Union['StoredList', 'StoredSet', 'StoredDict']
 
-    StoredObject = Union['StoredList', 'StoredSet', 'StoredDict']
-
-    _Path = _Kind = _MethodName = _EventKey = str
-    # used to type Framework Attributes
-    _ObserverPath = List[Tuple[_Path, _MethodName, _Path, _EventKey]]
-    _ObjectPath = Tuple[Optional[_Path], _Kind]
-    _PathToObjectMapping = Dict[_Path, 'Object']
-    _PathToSerializableMapping = Dict[_Path, Serializable]
+_Path = _Kind = _MethodName = _EventKey = str
+# used to type Framework Attributes
+_ObserverPath = List[Tuple[_Path, _MethodName, _Path, _EventKey]]
+_ObjectPath = Tuple[Optional[_Path], _Kind]
+_PathToObjectMapping = Dict[_Path, 'Object']
+_PathToSerializableMapping = Dict[_Path, Serializable]
 
 _T = TypeVar("_T")
 _EventType = TypeVar('_EventType', bound='EventBase')
@@ -561,21 +559,22 @@ class Framework(Object):
     model: 'Model' = None  # type: ignore
     """The :class:`Model` instance for this charm."""
 
-    meta: 'CharmMeta' = None  # type: ignore
+    meta: 'charm.CharmMeta' = None  # type: ignore
     """The charm's metadata."""
 
     charm_dir: 'pathlib.Path' = None  # type: ignore
     """The charm project root directory."""
 
+    _stored: 'StoredStateData' = None  # type: ignore
+
     # to help the type checker and IDEs:
     if TYPE_CHECKING:
-        _stored: 'StoredStateData' = None  # type: ignore
         @property
         def on(self) -> 'FrameworkEvents': ...  # noqa
 
     def __init__(self, storage: Union[SQLiteStorage, JujuStorage],
                  charm_dir: Union[str, pathlib.Path],
-                 meta: 'CharmMeta', model: 'Model',
+                 meta: 'charm.CharmMeta', model: 'Model',
                  event_name: Optional[str] = None):
         super().__init__(self, None)
 
@@ -1059,14 +1058,13 @@ def __init__(self, parent: Object, attr_name: str):
 
         parent.framework.observe(parent.framework.on.commit, self._data.on_commit)  # type: ignore
 
-    if TYPE_CHECKING:
-        @typing.overload
-        def __getattr__(self, key: Literal['on']) -> ObjectEvents:
-            pass
+    @typing.overload
+    def __getattr__(self, key: Literal['on']) -> ObjectEvents:
+        pass
 
-        @typing.overload
-        def __getattr__(self, key: str) -> Any:
-            pass
+    @typing.overload
+    def __getattr__(self, key: str) -> Any:
+        pass
 
     def __getattr__(self, key: str) -> Any:
         # "on" is the only reserved key that can't be used in the data map.
@@ -1125,20 +1123,19 @@ def __init__(self):
         self.parent_type: Optional[Type[Any]] = None
         self.attr_name: Optional[str] = None
 
-    if TYPE_CHECKING:
-        @typing.overload
-        def __get__(
-                self,
-                parent: Literal[None],
-                parent_type: 'Type[_ObjectType]') -> 'StoredState':
-            pass
-
-        @typing.overload
-        def __get__(
-                self,
-                parent: '_ObjectType',
-                parent_type: 'Type[_ObjectType]') -> BoundStoredState:
-            pass
+    @typing.overload
+    def __get__(
+            self,
+            parent: Literal[None],
+            parent_type: 'Type[_ObjectType]') -> 'StoredState':
+        pass
+
+    @typing.overload
+    def __get__(
+            self,
+            parent: '_ObjectType',
+            parent_type: 'Type[_ObjectType]') -> BoundStoredState:
+        pass
 
     def __get__(self,
                 parent: Optional['_ObjectType'],

ops/log.py

@@ -16,19 +16,16 @@
 
 import logging
 import sys
+import types
 import typing
 
-if typing.TYPE_CHECKING:
-    from types import TracebackType
-    from typing import Type
-
-    from ops.model import _ModelBackend
+from ops.model import _ModelBackend
 
 
 class JujuLogHandler(logging.Handler):
     """A handler for sending logs to Juju via juju-log."""
 
-    def __init__(self, model_backend: "_ModelBackend", level: int = logging.DEBUG):
+    def __init__(self, model_backend: _ModelBackend, level: int = logging.DEBUG):
         super().__init__(level)
         self.model_backend = model_backend
 
@@ -41,7 +38,7 @@ def emit(self, record: logging.LogRecord):
         self.model_backend.juju_log(record.levelname, self.format(record))
 
 
-def setup_root_logging(model_backend: "_ModelBackend", debug: bool = False):
+def setup_root_logging(model_backend: _ModelBackend, debug: bool = False):
     """Setup python logging to forward messages to juju-log.
 
     By default, logging is set to DEBUG level, and messages will be filtered by Juju.
@@ -62,7 +59,9 @@ def setup_root_logging(model_backend: "_ModelBackend", debug: bool = False):
         handler.setFormatter(formatter)
         logger.addHandler(handler)
 
-    def except_hook(etype: "Type[BaseException]", value: BaseException, tb: "TracebackType"):
+    def except_hook(etype: typing.Type[BaseException],
+                    value: BaseException,
+                    tb: types.TracebackType):
         logger.error(
             "Uncaught exception while in charm code:",
             exc_info=(etype, value, tb))

ops/main.py

@@ -26,7 +26,7 @@
 import sys
 import warnings
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Type, Union, cast
+from typing import Any, Dict, List, Optional, Tuple, Type, Union, cast
 
 import ops.charm
 import ops.framework
@@ -36,11 +36,6 @@
 from ops.jujuversion import JujuVersion
 from ops.log import setup_root_logging
 
-if TYPE_CHECKING:
-    from ops.charm import CharmBase
-    from ops.framework import BoundEvent, EventSource
-    from ops.model import Relation
-
 CHARM_STATE_FILE = '.unit-state.db'
 
 
@@ -68,7 +63,7 @@ def _get_charm_dir():
     return charm_dir
 
 
-def _create_event_link(charm: 'CharmBase', bound_event: 'EventSource',
+def _create_event_link(charm: 'ops.charm.CharmBase', bound_event: 'ops.framework.EventSource',
                        link_to: Union[str, Path]):
     """Create a symlink for a particular event.
 
@@ -106,7 +101,7 @@ def _create_event_link(charm: 'CharmBase', bound_event: 'EventSource',
         event_path.symlink_to(target_path)
 
 
-def _setup_event_links(charm_dir: Path, charm: 'CharmBase'):
+def _setup_event_links(charm_dir: Path, charm: 'ops.charm.CharmBase'):
     """Set up links for supported events that originate from Juju.
 
     Whether a charm can handle an event or not can be determined by
@@ -128,7 +123,7 @@ def _setup_event_links(charm_dir: Path, charm: 'CharmBase'):
             _create_event_link(charm, bound_event, link_to)
 
 
-def _emit_charm_event(charm: 'CharmBase', event_name: str):
+def _emit_charm_event(charm: 'ops.charm.CharmBase', event_name: str):
     """Emits a charm event based on a Juju event name.
 
     Args:
@@ -149,8 +144,8 @@ def _emit_charm_event(charm: 'CharmBase', event_name: str):
         event_to_emit.emit(*args, **kwargs)
 
 
-def _get_event_args(charm: 'CharmBase',
-                    bound_event: 'BoundEvent') -> Tuple[List[Any], Dict[str, Any]]:
+def _get_event_args(charm: 'ops.charm.CharmBase',
+                    bound_event: 'ops.framework.BoundEvent') -> Tuple[List[Any], Dict[str, Any]]:
     event_type = bound_event.event_type
     model = charm.framework.model
 
@@ -189,7 +184,7 @@ def _get_event_args(charm: 'CharmBase',
     elif issubclass(event_type, ops.charm.RelationEvent):
         relation_name = os.environ['JUJU_RELATION']
         relation_id = int(os.environ['JUJU_RELATION_ID'].split(':')[-1])
-        relation: Optional[Relation] = model.get_relation(relation_name, relation_id)
+        relation: Optional[ops.model.Relation] = model.get_relation(relation_name, relation_id)
 
     remote_app_name = os.environ.get('JUJU_REMOTE_APP', '')
     remote_unit_name = os.environ.get('JUJU_REMOTE_UNIT', '')
@@ -239,7 +234,7 @@ def __init__(self, charm_dir: Path):
         else:
             self._init_legacy()
 
-    def ensure_event_links(self, charm: 'CharmBase'):
+    def ensure_event_links(self, charm: 'ops.charm.CharmBase'):
         """Make sure necessary symlinks are present on disk."""
         if self.is_dispatch_aware:
             # links aren't needed