Skip to content

Commit

Permalink
feat: Add type hints (#256)
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald authored Aug 21, 2023
1 parent 14d42dd commit e0ee525
Show file tree
Hide file tree
Showing 25 changed files with 254 additions and 229 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 2.5.0

* Added python type hints

# 2.4.0

* Added Support for Django 4.2
Expand Down
6 changes: 6 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[mypy]
python_version = 3.8
follow_imports = normal
ignore_missing_imports = True
allow_untyped_globals = False
files = opaque_keys
78 changes: 39 additions & 39 deletions opaque_keys/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
an application, while concealing the particulars of the serialization
formats, and allowing new serialization formats to be installed transparently.
"""
from __future__ import annotations
from abc import ABCMeta, abstractmethod
from collections import defaultdict
from functools import total_ordering

# pylint: disable=wrong-import-order
from _collections import defaultdict
from stevedore.enabled import EnabledExtensionManager
from typing_extensions import Self # For python 3.11 plus, can just use "from typing import Self"

__version__ = '2.4.0'
__version__ = '2.5.0'


class InvalidKeyError(Exception):
Expand Down Expand Up @@ -94,15 +95,16 @@ class constructor will not validate any of the ``KEY_FIELDS`` arguments, and wil
"""
__slots__ = ('_initialized', 'deprecated')

KEY_FIELDS = []
CANONICAL_NAMESPACE = None
KEY_FIELDS: tuple[str, ...]
CANONICAL_NAMESPACE: str
KEY_TYPE: str
NAMESPACE_SEPARATOR = ':'
CHECKED_INIT = True
CHECKED_INIT: bool = True

# ============= ABSTRACT METHODS ==============
@classmethod
@abstractmethod
def _from_string(cls, serialized):
def _from_string(cls, serialized: str):
"""
Return an instance of `cls` parsed from its `serialized` form.
Expand All @@ -117,7 +119,7 @@ def _from_string(cls, serialized):
raise NotImplementedError()

@abstractmethod
def _to_string(self):
def _to_string(self) -> str:
"""
Return a serialization of `self`.
Expand All @@ -142,7 +144,7 @@ def _from_deprecated_string(cls, serialized):
InvalidKeyError: Should be raised if `serialized` is not a valid serialized key
understood by `cls`.
"""
raise NotImplementedError()
raise AttributeError("The specified key type does not have a deprecated version.")

def _to_deprecated_string(self):
"""
Expand All @@ -154,21 +156,21 @@ def _to_deprecated_string(self):
This serialization should not include the namespace prefix.
"""
raise NotImplementedError()
raise AttributeError("The specified key type does not have a deprecated version.")

# ============= SERIALIZATION ==============

def __str__(self):
def __str__(self) -> str:
"""
Serialize this :class:`OpaqueKey`, in the form ``<CANONICAL_NAMESPACE>:<value of _to_string>``.
"""
if self.deprecated:
# no namespace on deprecated
return self._to_deprecated_string()
return self.NAMESPACE_SEPARATOR.join([self.CANONICAL_NAMESPACE, self._to_string()]) # pylint: disable=no-member
return self.NAMESPACE_SEPARATOR.join([self.CANONICAL_NAMESPACE, self._to_string()])

@classmethod
def from_string(cls, serialized):
def from_string(cls, serialized: str) -> Self:
"""
Return a :class:`OpaqueKey` object deserialized from
the `serialized` argument. This object will be an instance
Expand All @@ -192,12 +194,12 @@ def from_string(cls, serialized):
raise InvalidKeyError(cls, serialized)
return key_class._from_string(rest)
except InvalidKeyError as error:
if hasattr(cls, 'deprecated_fallback') and issubclass(cls.deprecated_fallback, cls):
return cls.deprecated_fallback._from_deprecated_string(serialized)
if hasattr(cls, 'deprecated_fallback') and issubclass(cls.deprecated_fallback, cls): # type: ignore
return cls.deprecated_fallback._from_deprecated_string(serialized) # type: ignore
raise InvalidKeyError(cls, serialized) from error

@classmethod
def _separate_namespace(cls, serialized):
def _separate_namespace(cls, serialized: str):
"""
Return the namespace from a serialized :class:`OpaqueKey`, and
the rest of the key.
Expand All @@ -220,7 +222,7 @@ def _separate_namespace(cls, serialized):
return (namespace, rest)

@classmethod
def get_namespace_plugin(cls, namespace):
def get_namespace_plugin(cls, namespace: str):
"""
Return the registered OpaqueKey subclass of cls for the supplied namespace
"""
Expand All @@ -240,17 +242,17 @@ def get_namespace_plugin(cls, namespace):
# a particular unknown namespace (like i4x)
raise InvalidKeyError(cls, f'{namespace}:*') from error

LOADED_DRIVERS = defaultdict() # If you change default, change test_default_deprecated
LOADED_DRIVERS: dict[type[OpaqueKey], EnabledExtensionManager] = defaultdict()

@classmethod
def _drivers(cls):
def _drivers(cls: type[OpaqueKey]):
"""
Return a driver manager for all key classes that are
subclasses of `cls`.
"""
if cls not in cls.LOADED_DRIVERS:
cls.LOADED_DRIVERS[cls] = EnabledExtensionManager(
cls.KEY_TYPE, # pylint: disable=no-member
cls.KEY_TYPE,
check_func=lambda extension: issubclass(extension.plugin, cls),
invoke_on_load=False,
)
Expand All @@ -268,17 +270,16 @@ def set_deprecated_fallback(cls, fallback):
# ============= VALUE SEMANTICS ==============
def __init__(self, *args, **kwargs):
# The __init__ expects child classes to implement KEY_FIELDS
# pylint: disable=no-member

# a flag used to indicate that this instance was deserialized from the
# deprecated form and should serialize to the deprecated form
self.deprecated = kwargs.pop('deprecated', False) # pylint: disable=assigning-non-slot
self.deprecated = kwargs.pop('deprecated', False)

if self.CHECKED_INIT:
self._checked_init(*args, **kwargs)
else:
self._unchecked_init(**kwargs)
self._initialized = True # pylint: disable=assigning-non-slot
self._initialized = True

def _checked_init(self, *args, **kwargs):
"""
Expand Down Expand Up @@ -318,7 +319,7 @@ def replace(self, **kwargs):
Subclasses should override this if they have required properties that aren't included in their
``KEY_FIELDS``.
"""
existing_values = {key: getattr(self, key) for key in self.KEY_FIELDS} # pylint: disable=no-member
existing_values = {key: getattr(self, key) for key in self.KEY_FIELDS}
existing_values['deprecated'] = self.deprecated

if all(value == existing_values[key] for (key, value) in kwargs.items()):
Expand All @@ -331,7 +332,7 @@ def __setattr__(self, name, value):
if getattr(self, '_initialized', False):
raise AttributeError(f"Can't set {name!r}. OpaqueKeys are immutable.")

super().__setattr__(name, value) # pylint: disable=no-member
super().__setattr__(name, value)

def __delattr__(self, name):
raise AttributeError(f"Can't delete {name!r}. OpaqueKeys are immutable.")
Expand All @@ -352,44 +353,43 @@ def __deepcopy__(self, memo):
def __setstate__(self, state_dict):
# used by pickle to set fields on an unpickled object
for key in state_dict:
if key in self.KEY_FIELDS: # pylint: disable=no-member
if key in self.KEY_FIELDS:
setattr(self, key, state_dict[key])
self.deprecated = state_dict['deprecated'] # pylint: disable=assigning-non-slot
self._initialized = True # pylint: disable=assigning-non-slot
self.deprecated = state_dict['deprecated']
self._initialized = True

def __getstate__(self):
# used by pickle to get fields on an unpickled object
pickleable_dict = {}
for key in self.KEY_FIELDS: # pylint: disable=no-member
for key in self.KEY_FIELDS:
pickleable_dict[key] = getattr(self, key)
pickleable_dict['deprecated'] = self.deprecated
return pickleable_dict

@property
def _key(self):
def _key(self) -> tuple:
"""Returns a tuple of key fields"""
# pylint: disable=no-member
return tuple(getattr(self, field) for field in self.KEY_FIELDS) + (self.CANONICAL_NAMESPACE, self.deprecated)

def __eq__(self, other):
return isinstance(other, OpaqueKey) and self._key == other._key # pylint: disable=protected-access
def __eq__(self, other) -> bool:
return isinstance(other, OpaqueKey) and self._key == other._key

def __ne__(self, other):
def __ne__(self, other) -> bool:
return not self == other

def __lt__(self, other):
def __lt__(self, other) -> bool:
if (self.KEY_FIELDS, self.CANONICAL_NAMESPACE, self.deprecated) != (other.KEY_FIELDS, other.CANONICAL_NAMESPACE,
other.deprecated):
raise TypeError(f"{self!r} is incompatible with {other!r}")
return self._key < other._key # pylint: disable=protected-access
return self._key < other._key

def __hash__(self):
def __hash__(self) -> int:
return hash(self._key)

def __repr__(self):
def __repr__(self) -> str:
key_field_repr = ', '.join(repr(getattr(self, key)) for key in self.KEY_FIELDS)
return f'{self.__class__.__name__}({key_field_repr})'

def __len__(self):
def __len__(self) -> int:
"""Return the number of characters in the serialized OpaqueKey"""
return len(str(self))
14 changes: 6 additions & 8 deletions opaque_keys/edx/django/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Useful django models for implementing XBlock infrastructure in django.
If Django is unavailable, none of the classes below will work as intended.
"""
# pylint: disable=abstract-method
from __future__ import annotations
import logging
import warnings

Expand All @@ -16,6 +16,7 @@
CharField = object
IsNull = object

from opaque_keys import OpaqueKey
from opaque_keys.edx.keys import BlockTypeKey, CourseKey, LearningContextKey, UsageKey


Expand All @@ -41,7 +42,7 @@ def __set__(self, obj, value):
obj.__dict__[self.field.name] = self.field.to_python(value)


# pylint: disable=missing-docstring,unused-argument
# pylint: disable=unused-argument
class CreatorMixin:
"""
Mixin class to provide SubfieldBase functionality to django fields.
Expand Down Expand Up @@ -77,7 +78,6 @@ def _strip_value(value, lookup='exact'):
return stripped_value


# pylint: disable=logging-format-interpolation
class OpaqueKeyField(CreatorMixin, CharField):
"""
A django field for storing OpaqueKeys.
Expand All @@ -92,15 +92,15 @@ class OpaqueKeyField(CreatorMixin, CharField):
description = "An OpaqueKey object, saved to the DB in the form of a string."

Empty = object()
KEY_CLASS = None
KEY_CLASS: type[OpaqueKey]

def __init__(self, *args, **kwargs):
if self.KEY_CLASS is None:
raise ValueError('Must specify KEY_CLASS in OpaqueKeyField subclasses')

super().__init__(*args, **kwargs)

def to_python(self, value):
def to_python(self, value): # pylint: disable=missing-function-docstring
if value is self.Empty or value is None:
return None

Expand Down Expand Up @@ -128,13 +128,12 @@ def to_python(self, value):
return self.KEY_CLASS.from_string(value)
return value

def get_prep_value(self, value):
def get_prep_value(self, value): # pylint: disable=missing-function-docstring
if value is self.Empty or value is None:
return '' # CharFields should use '' as their empty value, rather than None

if isinstance(value, str):
value = self.KEY_CLASS.from_string(value)
# pylint: disable=isinstance-second-argument-not-valid-type
assert isinstance(value, self.KEY_CLASS), f"{value} is not an instance of {self.KEY_CLASS}"
serialized_key = str(_strip_value(value))
if serialized_key.endswith('\n'):
Expand Down Expand Up @@ -178,7 +177,6 @@ def get_prep_lookup(self):


try:
# pylint: disable=no-member
OpaqueKeyField.register_lookup(OpaqueKeyFieldEmptyLookupIsNull)
except AttributeError:
# Django was not imported
Expand Down
1 change: 0 additions & 1 deletion opaque_keys/edx/django/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def __eq__(self, obj):
return self.text == obj.text


# pylint: disable=missing-docstring
class ExampleField(CreatorMixin, CharField):
"""A simple Django Field to assist in testing the CreatorMixin class."""

Expand Down
7 changes: 3 additions & 4 deletions opaque_keys/edx/django/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def enable_db_access_for_all_tests(db):
"""Enable DB access for all tests."""


# pylint: disable=no-member
class TestCreatorMixin(TestCase):
"""Tests of the CreatorMixin class."""
def setUp(self):
Expand All @@ -33,7 +32,7 @@ def setUp(self):

def test_char_field_is_converted_to_container(self):
expected = Container('key-1').transform()
self.assertEqual(expected, self.model.key.transform())
self.assertEqual(expected, self.model.key.transform()) # pylint: disable=no-member

def test_load_model_from_db(self):
fetched_model = ExampleModel.objects.get(key='key-1')
Expand All @@ -48,8 +47,8 @@ class EmptyKeyClassField(OpaqueKeyField):
class TestOpaqueKeyField(TestCase):
"""Tests the implementation of OpaqueKeyField methods."""
def test_null_key_class_raises_value_error(self):
with self.assertRaises(ValueError):
EmptyKeyClassField()
with self.assertRaises(AttributeError):
EmptyKeyClassField() # AttributeError: 'EmptyKeyClassField' object has no attribute 'KEY_CLASS'

def test_to_python_trailing_newline_stripped(self):
field = ComplexModel()._meta.get_field('course_key')
Expand Down
Loading

0 comments on commit e0ee525

Please sign in to comment.