From 8a7c816a12d795cdd607144859091d0d1b8fb552 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Tue, 30 Jan 2024 13:23:23 -0500 Subject: [PATCH] feat: first pass at type-hints for core, fields, internal several TODOs in fields, still --- xblock/core.py | 78 ++++++------ xblock/fields.py | 291 +++++++++++++++++++++++++++------------------ xblock/internal.py | 20 +++- 3 files changed, 232 insertions(+), 157 deletions(-) diff --git a/xblock/core.py b/xblock/core.py index 2c93195ac..41e1d410e 100644 --- a/xblock/core.py +++ b/xblock/core.py @@ -3,14 +3,18 @@ This code is in the Runtime layer, because it is authored once by edX and used by all runtimes. - """ -from collections import defaultdict +from __future__ import annotations + import inspect import os import warnings +import typing as t +from collections import defaultdict import pkg_resources +from opaque_keys.edx.keys import LearningContextKey, UsageKey +from web_fragments.fragment import Fragment import xblock.exceptions from xblock.exceptions import DisallowedFileError @@ -53,26 +57,26 @@ class SharedBlockBase(Plugin): Behaviors and attrs which all XBlock like things should share """ - resources_dir = '' - public_dir = 'public' - i18n_js_namespace = None + resources_dir: str = '' + public_dir: str = 'public' + i18n_js_namespace: str | None = None @classmethod - def get_resources_dir(cls): + def get_resources_dir(cls) -> str: """ Gets the resource directory for this XBlock. """ return cls.resources_dir @classmethod - def get_public_dir(cls): + def get_public_dir(cls) -> str: """ Gets the public directory for this XBlock. """ return cls.public_dir @classmethod - def get_i18n_js_namespace(cls): + def get_i18n_js_namespace(cls) -> str | None: """ Gets the JavaScript translations namespace for this XBlock. @@ -83,7 +87,7 @@ def get_i18n_js_namespace(cls): return cls.i18n_js_namespace @classmethod - def open_local_resource(cls, uri): + def open_local_resource(cls, uri: str) -> t.BinaryIO: """ Open a local resource. @@ -125,21 +129,21 @@ def open_local_resource(cls, uri): # -- Base Block class XBlock(XmlSerializationMixin, HierarchyMixin, ScopedStorageMixin, RuntimeServicesMixin, HandlersMixin, IndexInfoMixin, ViewsMixin, SharedBlockBase): - """Base class for XBlocks. + """ + Base class for XBlocks. Derive from this class to create a new kind of XBlock. There are no required methods, but you will probably need at least one view. Don't provide the ``__init__`` method when deriving from this class. - """ - entry_point = 'xblock.v1' + entry_point: str = 'xblock.v1' name = String(help="Short name for the block", scope=Scope.settings) tags = List(help="Tags for this block", scope=Scope.settings) @class_lazy - def _class_tags(cls): # pylint: disable=no-self-argument + def _class_tags(cls: type[XBlock]) -> set[str]: # pylint: disable=no-self-argument """ Collect the tags from all base classes. """ @@ -153,7 +157,7 @@ def _class_tags(cls): # pylint: disable=no-self-argument @staticmethod def tag(tags): """Returns a function that adds the words in `tags` as class tags to this class.""" - def dec(cls): + def dec(cls: type[XBlock]) -> type[XBlock]: """Add the words in `tags` as class tags to this class.""" # Add in this class's tags cls._class_tags.update(tags.replace(",", " ").split()) # pylint: disable=protected-access @@ -161,7 +165,7 @@ def dec(cls): return dec @classmethod - def load_tagged_classes(cls, tag, fail_silently=True): + def load_tagged_classes(cls, tag, fail_silently=True) -> t.Iterable[type[XBlock]]: """ Produce a sequence of all XBlock classes tagged with `tag`. @@ -181,7 +185,14 @@ def load_tagged_classes(cls, tag, fail_silently=True): yield name, class_ # pylint: disable=keyword-arg-before-vararg - def __init__(self, runtime, field_data=None, scope_ids=UNSET, *args, **kwargs): + def __init__( + self, + runtime: Runtime, + field_data: FieldData | None = None, + scope_ids: ScopeIds = UNSET, + *args, + **kwargs + ): """ Construct a new XBlock. @@ -206,7 +217,7 @@ def __init__(self, runtime, field_data=None, scope_ids=UNSET, *args, **kwargs): super().__init__(runtime=runtime, scope_ids=scope_ids, field_data=field_data, *args, **kwargs) @property - def usage_key(self): + def usage_key(self) -> UsageKey: """ A key identifying this particular usage of the XBlock, unique across all learning contexts in the system. @@ -215,7 +226,7 @@ def usage_key(self): return self.scope_ids.usage_id @property - def context_key(self): + def context_key(self) -> LearningContextKey | None: """ A key identifying the learning context (course, library, etc.) that contains this XBlock usage. @@ -231,11 +242,11 @@ def context_key(self): """ return getattr(self.scope_ids.usage_id, "context_key", None) - def render(self, view, context=None): + def render(self, view: str, context: dict | None = None) -> Fragment: """Render `view` with this block's runtime and the supplied `context`""" return self.runtime.render(self, view, context) - def validate(self): + def validate(self) -> Validation: """ Ask this xblock to validate itself. Subclasses are expected to override this method, as there is currently only a no-op implementation. Any overriding method @@ -244,7 +255,7 @@ def validate(self): """ return Validation(self.scope_ids.usage_id) - def ugettext(self, text): + def ugettext(self, text) -> str: """ Translates message/text and returns it in a unicode string. Using runtime to get i18n service. @@ -253,7 +264,7 @@ def ugettext(self, text): runtime_ugettext = runtime_service.ugettext return runtime_ugettext(text) - def add_xml_to_node(self, node): + def add_xml_to_node(self, node: etree.Element) -> None: """ For exporting, set data on etree.Element `node`. """ @@ -262,15 +273,18 @@ def add_xml_to_node(self, node): self.add_children_to_node(node) +XBlockAsideView: t.TypeAlias = t.Callable[[XBlockAside, XBlock, dict | None], Fragment] + + class XBlockAside(XmlSerializationMixin, ScopedStorageMixin, RuntimeServicesMixin, HandlersMixin, SharedBlockBase): """ This mixin allows Xblock-like class to declare that it provides aside functionality. """ - entry_point = "xblock_asides.v1" + entry_point: str = "xblock_asides.v1" @classmethod - def aside_for(cls, view_name): + def aside_for(cls, view_name: str) -> t.Callable[[XBlockAsideView], XBlockAsideView]: """ A decorator to indicate a function is the aside view for the given view_name. @@ -283,7 +297,7 @@ def student_aside(self, block, context=None): """ # pylint: disable=protected-access - def _decorator(func): + def _decorator(func: XBlockAsideView) -> XBlockAsideView: if not hasattr(func, '_aside_for'): func._aside_for = [] @@ -292,7 +306,7 @@ def _decorator(func): return _decorator @classmethod - def should_apply_to_block(cls, block): # pylint: disable=unused-argument + def should_apply_to_block(cls, block: XBlock) -> bool: # pylint: disable=unused-argument """ Return True if the aside should be applied to a given block. This can be overridden if some aside should only wrap blocks with certain properties. @@ -300,7 +314,7 @@ def should_apply_to_block(cls, block): # pylint: disable=unused-argument return True @class_lazy - def _combined_asides(cls): # pylint: disable=no-self-argument + def _combined_asides(cls) -> dict[str, str | None]: # pylint: disable=no-self-argument """ A dictionary mapping XBlock view names to the aside method that decorates them (or None, if there is no decorator for the specified view). @@ -314,25 +328,19 @@ def _combined_asides(cls): # pylint: disable=no-self-argument combined_asides[view] = view_func.__name__ return combined_asides - def aside_view_declaration(self, view_name): + def aside_view_declaration(self, view_name: str) -> XBlockAsideView | None: """ Find and return a function object if one is an aside_view for the given view_name Aside methods declare their view provision via @XBlockAside.aside_for(view_name) This function finds those declarations for a block. - - Arguments: - view_name (str): the name of the view requested. - - Returns: - either the function or None """ if view_name in self._combined_asides: # pylint: disable=unsupported-membership-test return getattr(self, self._combined_asides[view_name]) # pylint: disable=unsubscriptable-object else: return None - def needs_serialization(self): + def needs_serialization(self) -> bool: """ Return True if the aside has any data to serialize to XML. diff --git a/xblock/fields.py b/xblock/fields.py index 8f5eecc8a..57caa62db 100644 --- a/xblock/fields.py +++ b/xblock/fields.py @@ -5,7 +5,8 @@ for each scope. """ -from collections import namedtuple +from __future__ import annotations + import copy import datetime import hashlib @@ -13,15 +14,23 @@ import json import re import traceback +import typing as t import warnings +from dataclasses import dataclass +from enum import Enum import dateutil.parser -from lxml import etree import pytz import yaml +from lxml import etree +from opaque_keys.edx.keys import DefinitionKey, UsageKey from xblock.internal import Nameable +if t.TYPE_CHECKING: + from .core import XBlock + + # __all__ controls what classes end up in the docs, and in what order. __all__ = [ 'BlockScope', 'UserScope', 'Scope', 'ScopeIds', @@ -43,40 +52,43 @@ class ModifyingEnforceTypeWarning(DeprecationWarning): """ +@dataclass(frozen=True) class Sentinel: """ Class for implementing sentinel objects (only equal to themselves). """ + name: str = "" # Name is 'optional' for ease of making sub-class instances. - def __init__(self, name): + def __post_init__(self): """ - `name` is the name used to identify the sentinel (which will - be displayed as the __repr__) of the sentinel. + Ensure `self.name` is set, either by constructor or subclass. """ - self.name = name + if not self.name: + raise ValueError("Coding error: sentinel must have a name!") - def __repr__(self): + def __repr__(self) -> str: return self.name @property - def attr_name(self): + def attr_name(self) -> str: """ TODO: Look into namespace collisions. block.name_space == block_name.space """ return self.name.lower().replace('.', '_') - def __eq__(self, other): - """ Equality is based on being of the same class, and having same name + def __eq__(self, other: object) -> bool: """ - return isinstance(other, Sentinel) and self.name == other.name + Equality is based on being of the same class, and having same name + """ + return isinstance(other, self.__class__) and self.name == other.name - def __hash__(self): + def __hash__(self) -> int: """ Use a hash of the name of the sentinel """ return hash(self.name) -class BlockScope: +class BlockScope(Enum): """ Enumeration of block scopes. @@ -96,23 +108,20 @@ class BlockScope: information that is purely about the student. """ - USAGE = Sentinel('BlockScope.USAGE') - DEFINITION = Sentinel('BlockScope.DEFINITION') - TYPE = Sentinel('BlockScope.TYPE') - ALL = Sentinel('BlockScope.ALL') + USAGE = 'BlockScope.USAGE' + DEFINITION = 'BlockScope.DEFINITION' + TYPE = 'BlockScope.TYPE' + ALL = 'BlockScope.ALL' @classmethod - def scopes(cls): + def scopes(cls) -> list[BlockScope]: """ Return a list of valid/understood class scopes. """ - # Why do we need this? This should either - # * Be bubbled to the places where it is used (AcidXBlock). - # * Be automatic. Look for all members of a type. - return [cls.USAGE, cls.DEFINITION, cls.TYPE, cls.ALL] + return list(cls) -class UserScope: +class UserScope(Enum): """ Enumeration of user scopes. @@ -133,22 +142,23 @@ class UserScope: submitted by all students. """ - NONE = Sentinel('UserScope.NONE') - ONE = Sentinel('UserScope.ONE') - ALL = Sentinel('UserScope.ALL') + NONE = 'UserScope.NONE' + ONE = 'UserScope.ONE' + ALL = 'UserScope.ALL' @classmethod - def scopes(cls): + def scopes(cls) -> list[UserScope]: """ Return a list of valid/understood class scopes. Why do we need this? I believe it is not used anywhere. """ - return [cls.NONE, cls.ONE, cls.ALL] - + return list(cls) -UNSET = Sentinel("fields.UNSET") -ScopeBase = namedtuple('ScopeBase', 'user block name') +class ScopeBase(t.NamedTuple): + user: UserScope + block: BlockScope + name: str class Scope(ScopeBase): @@ -184,15 +194,15 @@ class Scope(ScopeBase): the points scored by all users attempting a problem. """ - content = ScopeBase(UserScope.NONE, BlockScope.DEFINITION, 'content') - settings = ScopeBase(UserScope.NONE, BlockScope.USAGE, 'settings') - user_state = ScopeBase(UserScope.ONE, BlockScope.USAGE, 'user_state') - preferences = ScopeBase(UserScope.ONE, BlockScope.TYPE, 'preferences') - user_info = ScopeBase(UserScope.ONE, BlockScope.ALL, 'user_info') - user_state_summary = ScopeBase(UserScope.ALL, BlockScope.USAGE, 'user_state_summary') + content: t.ClassVar = ScopeBase(UserScope.NONE, BlockScope.DEFINITION, 'content') + settings: t.ClassVar = ScopeBase(UserScope.NONE, BlockScope.USAGE, 'settings') + user_state: t.ClassVar = ScopeBase(UserScope.ONE, BlockScope.USAGE, 'user_state') + preferences: t.ClassVar = ScopeBase(UserScope.ONE, BlockScope.TYPE, 'preferences') + user_info: t.ClassVar = ScopeBase(UserScope.ONE, BlockScope.ALL, 'user_info') + user_state_summary: t.ClassVar = ScopeBase(UserScope.ALL, BlockScope.USAGE, 'user_state_summary') @classmethod - def named_scopes(cls): + def named_scopes(cls) -> list[ScopeBase]: """Return all named Scopes.""" return [ cls.content, @@ -204,7 +214,7 @@ def named_scopes(cls): ] @classmethod - def scopes(cls): + def scopes(cls) -> list[ScopeBase]: """Return all possible Scopes.""" named_scopes = cls.named_scopes() return named_scopes + [ @@ -214,56 +224,94 @@ def scopes(cls): if cls(user, block) not in named_scopes ] - def __new__(cls, user, block, name=None): + def __new__(cls, user: UserScope, block, name: str | None = None) -> Scope: """Create a new Scope, with an optional name.""" + return cls(user, block, name or f'{user}_{block}') - if name is None: - name = f'{user}_{block}' - - return ScopeBase.__new__(cls, user, block, name) + children: t.ClassVar = Sentinel('Scope.children') + parent: t.ClassVar = Sentinel('Scope.parent') - children = Sentinel('Scope.children') - parent = Sentinel('Scope.parent') - - def __str__(self): + def __str__(self) -> str: return self.name - def __eq__(self, other): + def __eq__(self, other: object) -> bool: return isinstance(other, Scope) and self.user == other.user and self.block == other.block - def __hash__(self): + def __hash__(self) -> int: return hash(('xblock.fields.Scope', self.user, self.block)) -class ScopeIds(namedtuple('ScopeIds', 'user_id block_type def_id usage_id')): +class ScopeIds(t.NamedTuple): """ A simple wrapper to collect all of the ids needed to correctly identify an XBlock (or other classes deriving from ScopedStorageMixin) to a FieldData. These identifiers match up with BlockScope and UserScope attributes, so that, for instance, the `def_id` identifies scopes that use BlockScope.DEFINITION. """ - __slots__ = () + user_id: int | str | None + block_type: str + def_id: DefinitionKey + usage_id: UsageKey + + +ScopeIds.__slots__ = () # type: ignore + + +@dataclass(frozen=True) +class Unset(Sentinel): + """ + Indicates that default value has not been provided. + """ + name = "fields.UNSET" -# Define special reference that can be used as a field's default in field -# definition to signal that the field should default to a unique string value -# calculated at runtime. -UNIQUE_ID = Sentinel("fields.UNIQUE_ID") +@dataclass(frozen=True) +class UniqueId(Sentinel): + """ + A special reference that can be used as a field's default in field + definition to signal that the field should default to a unique string value + calculated at runtime. + """ + name = "fields.UNIQUE_ID" -# define a placeholder ('nil') value to indicate when nothing has been stored -# in the cache ("None" may be a valid value in the cache, so we cannot use it). -NO_CACHE_VALUE = Sentinel("fields.NO_CACHE_VALUE") -# define a placeholder value that indicates that a value is explicitly dirty, -# because it was explicitly set -EXPLICITLY_SET = Sentinel("fields.EXPLICITLY_SET") +@dataclass(frozen=True) +class NoCacheValue(Sentinel): + """ + Placeholder ('nil') value to indicate when nothing has been stored + in the cache ("None" may be a valid value in the cache, so we cannot use it). + """ + name = "fields.NO_CACHE_VALUE" + + +@dataclass(frozen=True) +class ExplicitlySet(Sentinel): + """ + Placeholder value that indicates that a value is explicitly dirty, + because it was explicitly set. + """ + name = "fields.EXPLICITLY_SET" + + +# For backwards API compatibility, define an instance of each Field-related sentinel. +# Because our sentinels are now frozen dataclasses (i.e., value objects), these could be +# be removed in favor of just constructing the class. For example, every use of +# `UNIQUE_ID` could be replaced with `UniqueId()`. +UNSET = Unset() +UNIQUE_ID = UniqueId() +NO_CACHE_VALUE = NoCacheValue() +EXPLICITLY_SET = ExplicitlySet() + # Fields that cannot have runtime-generated defaults. These are special, # because they define the structure of XBlock trees. NO_GENERATED_DEFAULTS = ('parent', 'children') -class Field(Nameable): +FieldValue = t.TypeVar("FieldValue") + + +class Field(Nameable, t.Generic[FieldValue]): """ A field class that can be used as a class attribute to define what data the class will want to refer to. @@ -307,21 +355,30 @@ class will want to refer to. runtime_options. """ - MUTABLE = True - _default = None - # Indicates if a field's None value should be sent to the XML representation. - none_to_xml = False + MUTABLE: bool = True + _default: FieldValue | None | UniqueId = None - # We're OK redefining built-in `help` - def __init__(self, help=None, default=UNSET, scope=Scope.content, # pylint:disable=redefined-builtin - display_name=None, values=None, enforce_type=False, - xml_node=False, force_export=False, **kwargs): + # Indicates if a field's None value should be sent to the XML representation. + none_to_xml: bool = False + + def __init__( + self, + help: str | None = None, # pylint:disable=redefined-builtin + default: FieldValue | None | UniqueId | Unset = Unset(), + scope: ScopeBase = Scope.content, + display_name: str | None = None, + values: list[object] | None = None, + enforce_type: bool = False, + xml_node: bool = False, + force_export: bool = False, + **kwargs + ): self.warned = False self.help = help self._enable_enforce_type = enforce_type - if default is not UNSET: - if default is UNIQUE_ID: - self._default = UNIQUE_ID + if not isinstance(default, Unset): + if isinstance(default, UniqueId): + self._default = UniqueId() else: self._default = self._check_or_enforce_type(default) self.scope = scope @@ -332,7 +389,7 @@ def __init__(self, help=None, default=UNSET, scope=Scope.content, # pylint:disa self.force_export = force_export @property - def default(self): + def default(self) -> FieldValue | None | UniqueId: """Returns the static value that this defaults to.""" if self.MUTABLE: return copy.deepcopy(self._default) @@ -340,13 +397,13 @@ def default(self): return self._default @property - def name(self): + def name(self) -> str: """Returns the name of this field.""" # This is set by ModelMetaclass return self.__name__ or 'unknown' @property - def values(self): + def values(self) -> t.Any: """ Returns the valid values for this class. This is useful for representing possible values in a UI. @@ -379,7 +436,7 @@ def values(self): return self._values @property - def display_name(self): + def display_name(self) -> str: """ Returns the display name for this class, suitable for use in a GUI. @@ -387,27 +444,27 @@ def display_name(self): """ return self._display_name if self._display_name is not None else self.name - def _get_cached_value(self, xblock): + def _get_cached_value(self, xblock) -> FieldValue | None | NoCacheValue: """ Return a value from the xblock's cache, or a marker value if either the cache doesn't exist or the value is not found in the cache. """ - return getattr(xblock, '_field_data_cache', {}).get(self.name, NO_CACHE_VALUE) + return getattr(xblock, '_field_data_cache', {}).get(self.name, NoCacheValue()) - def _set_cached_value(self, xblock, value): + def _set_cached_value(self, xblock: XBlock, value: FieldValue | None): """Store a value in the xblock's cache, creating the cache if necessary.""" # pylint: disable=protected-access if not hasattr(xblock, '_field_data_cache'): xblock._field_data_cache = {} xblock._field_data_cache[self.name] = value - def _del_cached_value(self, xblock): + def _del_cached_value(self, xblock: XBlock): """Remove a value from the xblock's cache, if the cache exists.""" # pylint: disable=protected-access if hasattr(xblock, '_field_data_cache') and self.name in xblock._field_data_cache: del xblock._field_data_cache[self.name] - def _mark_dirty(self, xblock, value): + def _mark_dirty(self, xblock: XBlock, value: FieldValue | None | ExplicitlySet): """Set this field to dirty on the xblock.""" # pylint: disable=protected-access @@ -416,7 +473,7 @@ def _mark_dirty(self, xblock, value): if self not in xblock._dirty_fields: xblock._dirty_fields[self] = copy.deepcopy(value) - def _is_dirty(self, xblock): + def _is_dirty(self, xblock: XBlock) -> bool: """ Return whether this field should be saved when xblock.save() is called """ @@ -425,15 +482,15 @@ def _is_dirty(self, xblock): return False baseline = xblock._dirty_fields[self] - return baseline is EXPLICITLY_SET or xblock._field_data_cache[self.name] != baseline + return isinstance(baseline, ExplicitlySet) or xblock._field_data_cache[self.name] != baseline - def _is_lazy(self, value): + def _is_lazy(self, value: FieldValue | None) -> bool: """ Detect if a value is being evaluated lazily by Django. """ return 'django.utils.functional.' in str(type(value)) - def _check_or_enforce_type(self, value): + def _check_or_enforce_type(self, value: t.Any) -> FieldValue | None: """ Depending on whether enforce_type is enabled call self.enforce_type and return the result or call it and trigger a silent warning if the result @@ -462,9 +519,9 @@ def _check_or_enforce_type(self, value): value, new_value) warnings.warn(message, ModifyingEnforceTypeWarning, stacklevel=3) - return value + return value # type: ignore - def _calculate_unique_id(self, xblock): + def _calculate_unique_id(self, xblock: XBlock) -> str: """ Provide a default value for fields with `default=UNIQUE_ID`. @@ -474,7 +531,7 @@ def _calculate_unique_id(self, xblock): key = scope_key(self, xblock) return hashlib.sha1(key.encode('utf-8')).hexdigest() - def _get_default_value_to_cache(self, xblock): + def _get_default_value_to_cache(self, xblock: XBlock) -> FieldValue | None: """ Perform special logic to provide a field's default value for caching. """ @@ -482,39 +539,42 @@ def _get_default_value_to_cache(self, xblock): # pylint: disable=protected-access return self.from_json(xblock._field_data.default(xblock, self.name)) except KeyError: - if self._default is UNIQUE_ID: + if isinstance(self.default, UniqueId): return self._check_or_enforce_type(self._calculate_unique_id(xblock)) else: return self.default - def _sanitize(self, value): + def _sanitize(self, value: FieldValue | None) -> FieldValue | None: """ Allow the individual fields to sanitize the value being set -or- "get". For example, a String field wants to remove control characters. """ return value - def __get__(self, xblock, xblock_class): + def __get__(self, xblock: XBlock, xblock_class: type[XBlock]) -> FieldValue | None: """ Gets the value of this xblock. Prioritizes the cached value over obtaining the value from the field-data service. Thus if a cached value exists, that is the value that will be returned. """ if xblock is None: - return self + return self # TODO: wtf? field_data = xblock._field_data - value = self._get_cached_value(xblock) - if value is NO_CACHE_VALUE: + cache_value = self._get_cached_value(xblock) + value: FieldValue | None + if isinstance(cache_value, NoCacheValue): if field_data.has(xblock, self.name): value = self.from_json(field_data.get(xblock, self.name)) elif self.name not in NO_GENERATED_DEFAULTS: # Cache default value value = self._get_default_value_to_cache(xblock) - else: - value = self.default + else: # TODO: why aren't we handling UniqueId here??? + value = self.default # type: ignore self._set_cached_value(xblock, value) + else: + value = cache_value # If this is a mutable type, mark it as dirty, since mutations can occur without an # explicit call to __set__ (but they do require a call to __get__) @@ -523,7 +583,7 @@ def __get__(self, xblock, xblock_class): return self._sanitize(value) - def __set__(self, xblock, value): + def __set__(self, xblock: XBlock, value: FieldValue | None) -> None: """ Sets the `xblock` to the given `value`. Setting a value does not update the underlying data store; the @@ -535,7 +595,7 @@ def __set__(self, xblock, value): """ value = self._check_or_enforce_type(value) value = self._sanitize(value) - cached_value = self._get_cached_value(xblock) + cached_value = self._get_cached_value(xblock) # TODO: why aren't we handling NoCacheValue?? try: value_has_changed = cached_value != value except Exception: # pylint: disable=broad-except @@ -544,10 +604,10 @@ def __set__(self, xblock, value): value_has_changed = True if value_has_changed: # Mark the field as dirty and update the cache - self._mark_dirty(xblock, EXPLICITLY_SET) + self._mark_dirty(xblock, ExplicitlySet()) self._set_cached_value(xblock, value) - def __delete__(self, xblock): + def __delete__(self, xblock: XBlock): """ Deletes `xblock` from the underlying data store. Deletes are not cached; they are performed immediately. @@ -591,7 +651,7 @@ def _warn_deprecated_outside_JSONField(self): # pylint: disable=invalid-name ) self.warned = True - def to_json(self, value): + def to_json(self, value: FieldValue | None) -> FieldValue | None: """ Return value in the form of nested lists and dictionaries (suitable for passing to json.dumps). @@ -602,7 +662,7 @@ def to_json(self, value): self._warn_deprecated_outside_JSONField() return value - def from_json(self, value): + def from_json(self, value: FieldValue | None) -> FieldValue | None: """ Return value as a native full featured python type (the inverse of to_json) @@ -612,20 +672,19 @@ def from_json(self, value): self._warn_deprecated_outside_JSONField() return value - def to_string(self, value): + def to_string(self, value: FieldValue | None) -> str: """ Return a JSON serialized string representation of the value. """ self._warn_deprecated_outside_JSONField() - value = json.dumps( + return json.dumps( self.to_json(value), indent=2, sort_keys=True, separators=(',', ': '), ) - return value - def from_string(self, serialized): + def from_string(self, serialized: str) -> FieldValue | None: """ Returns a native value from a YAML serialized string representation. Since YAML is a superset of JSON, this is the inverse of to_string.) @@ -634,7 +693,7 @@ def from_string(self, serialized): value = yaml.safe_load(serialized) return self.enforce_type(value) - def enforce_type(self, value): + def enforce_type(self, value: t.Any) -> FieldValue | None: """ Coerce the type of the value, if necessary @@ -644,41 +703,41 @@ def enforce_type(self, value): This must not have side effects, since it will be executed to trigger a DeprecationWarning even if enforce_type is disabled """ - return value + return value # type: ignore - def read_from(self, xblock): + def read_from(self, xblock: XBlock) -> FieldValue | None: """ Retrieve the value for this field from the specified xblock """ return self.__get__(xblock, xblock.__class__) # pylint: disable=unnecessary-dunder-call - def read_json(self, xblock): + def read_json(self, xblock: XBlock) -> FieldValue | None: """ Retrieve the serialized value for this field from the specified xblock """ self._warn_deprecated_outside_JSONField() return self.to_json(self.read_from(xblock)) - def write_to(self, xblock, value): + def write_to(self, xblock: XBlock, value: FieldValue | None) -> None: """ Set the value for this field to value on the supplied xblock """ self.__set__(xblock, value) # pylint: disable=unnecessary-dunder-call - def delete_from(self, xblock): + def delete_from(self, xblock: XBlock) -> None: """ Delete the value for this field from the supplied xblock """ self.__delete__(xblock) # pylint: disable=unnecessary-dunder-call - def is_set_on(self, xblock): + def is_set_on(self, xblock: XBlock) -> bool: """ Return whether this field has a non-default value on the supplied xblock """ # pylint: disable=protected-access return self._is_dirty(xblock) or xblock._field_data.has(xblock, self.name) - def __hash__(self): + def __hash__(self) -> int: return hash(self.name) diff --git a/xblock/internal.py b/xblock/internal.py index 12632aff0..62f725bc5 100644 --- a/xblock/internal.py +++ b/xblock/internal.py @@ -1,8 +1,11 @@ """ Internal machinery used to make building XBlock family base classes easier. """ +from __future__ import annotations + import functools import inspect +import typing as t class LazyClassProperty: @@ -13,9 +16,9 @@ class LazyClassProperty: executing the decorated method once, and then storing the result in the class __dict__. """ - def __init__(self, constructor): - self.__constructor = constructor - self.__cache = {} + def __init__(self, constructor: t.Callable): + self.__constructor: t.Callable = constructor + self.__cache: dict[object, object] = {} functools.wraps(self.__constructor)(self) def __get__(self, instance, owner): @@ -35,7 +38,12 @@ class NamedAttributesMetaclass(type): A metaclass which adds the __name__ attribute to all Nameable attributes which are attributes of the instantiated class, or of its baseclasses. """ - def __new__(mcs, name, bases, attrs): + def __new__( + mcs: type[NamedAttributesMetaclass], + name: str, + bases: tuple[type, ...], + attrs: dict[str, t.Any], + ): # Iterate over the attrs before they're bound to the class # so that we don't accidentally trigger any __get__ methods for attr_name, attr in attrs.items(): @@ -58,10 +66,10 @@ class Nameable: :class:`.NamedAttributesMetaclass`, will be assigned a `__name__` attribute based on what class attribute they are bound to. """ - __name__ = None + __name__: str | None = None @staticmethod - def needs_name(obj): + def needs_name(obj: object) -> bool: """ Return True if `obj` is a :class:`.Nameable` object that hasn't yet been assigned a name.