Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type-Hints später verwenden #147

Merged
merged 2 commits into from
Feb 11, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -6,6 +6,8 @@


class SinglechoiceAttempt(Attempt):
question: "SinglechoiceQuestion"

def _compute_score(self) -> float:
if not self.response or "choice" not in self.response:
msg = "'choice' is missing"
9 changes: 3 additions & 6 deletions questionpy/_attempt.py
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@
from questionpy_common.api.attempt import AttemptFile, AttemptUi, CacheControl, ScoredInputModel, ScoringCode

from ._ui import create_jinja2_environment
from ._util import get_mro_type_hint
from ._util import reify_type_hint

if TYPE_CHECKING:
from ._qtype import Question
@@ -138,8 +138,8 @@ class Attempt(ABC):
attempt_state: BaseAttemptState
scoring_state: BaseScoringState | None

attempt_state_class: ClassVar[type[BaseAttemptState]]
scoring_state_class: ClassVar[type[BaseScoringState]]
attempt_state_class: ClassVar[type[BaseAttemptState]] = reify_type_hint("attempt_state", BaseAttemptState)
scoring_state_class: ClassVar[type[BaseScoringState]] = reify_type_hint("scoring_state", BaseScoringState)

def __init__(
self,
@@ -246,9 +246,6 @@ def variant(self) -> int:
def __init_subclass__(cls, *args: object, **kwargs: object):
super().__init_subclass__(*args, **kwargs)

cls.attempt_state_class = get_mro_type_hint(cls, "attempt_state", BaseAttemptState)
cls.scoring_state_class = get_mro_type_hint(cls, "scoring_state", BaseScoringState)


class _ScoringError(Exception):
def __init__(self, scoring_code: ScoringCode, *args: object) -> None:
22 changes: 6 additions & 16 deletions questionpy/_qtype.py
Original file line number Diff line number Diff line change
@@ -11,9 +11,8 @@
from questionpy_common.environment import get_qpy_environment

from ._attempt import Attempt, AttemptProtocol, AttemptScoredProtocol, AttemptStartedProtocol
from ._util import get_mro_type_hint
from ._util import cached_class_property, reify_type_hint
from .form import FormModel, OptionsFormDefinition
from .form.validation import validate_form

_F = TypeVar("_F", bound=FormModel)
_S = TypeVar("_S", bound="BaseQuestionState")
@@ -36,9 +35,11 @@ class Question(ABC):
options: FormModel
question_state: BaseQuestionState

options_class: ClassVar[type[FormModel]]
question_state_class: ClassVar[type[BaseQuestionState]]
question_state_with_version_class: ClassVar[type[QuestionStateWithVersion]]
options_class: ClassVar[type[FormModel]] = reify_type_hint("options", FormModel)
question_state_class: ClassVar[type[BaseQuestionState]] = reify_type_hint("question_state", BaseQuestionState)
question_state_with_version_class: ClassVar[type[QuestionStateWithVersion]] = cached_class_property(
lambda cls: QuestionStateWithVersion[cls.options_class, cls.question_state_class] # type: ignore[name-defined]
)
Comment on lines +40 to +42
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: Keine Verwendung als Decorator (@cached_class_property), weil Mypy das nur versteht, wenn auch @classmethod dran ist, was dann wieder support in cached_class_property braucht, etc. So war es einfacher.


def __init__(self, qswv: QuestionStateWithVersion) -> None:
self.question_state_with_version = qswv
@@ -164,17 +165,6 @@ def __init_subclass__(cls, *args: object, **kwargs: object) -> None:
msg = f"Missing '{cls.__name__}.attempt_class' attribute. It should point to your attempt implementation"
raise TypeError(msg)

cls.question_state_class = get_mro_type_hint(cls, "question_state", BaseQuestionState)
cls.options_class = get_mro_type_hint(cls, "options", FormModel)
cls.question_state_with_version_class = QuestionStateWithVersion[ # type: ignore[misc]
cls.options_class, cls.question_state_class # type: ignore[name-defined]
]

# A form may have unresolved references when it is intended to be used as a repetition, group, section, etc.
# Only the complete form must pass validation, so the validation has to happen here instead of in FormModel or
# OptionsFormDefinition.
validate_form(cls.options_class.qpy_form)

@property # type: ignore[no-redef]
def options(self) -> FormModel:
return self.question_state_with_version.options
60 changes: 56 additions & 4 deletions questionpy/_util.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,63 @@
from collections.abc import Callable
from types import UnionType
from typing import TypeVar, get_args, get_type_hints
from typing import Generic, TypeVar, cast, get_args, get_type_hints

_T = TypeVar("_T", bound=type)
_TypeT = TypeVar("_TypeT", bound=type)
_T = TypeVar("_T")
_UNSET = object()


def get_mro_type_hint(klass: type, attr_name: str, bound: _T) -> _T:
# FIXME: This function is called too early, when the classes referenced in forward refs may not be defined yet.
class _CachedClassProperty(Generic[_TypeT, _T]):
"""See [cached_class_property][]."""

def __init__(self, getter: Callable[[_TypeT], _T]) -> None:
self._getter = getter
self._name: str | None = None

def __get__(self, instance: None, owner: _TypeT) -> _T:
if not self._name:
# __set_name__ wasn't called. The property is probably not defined in the class body, or wrapped by
# something.
return self._getter(owner)

# Subsequent lookups _should_ bypass the property entirely, but if for some reason they don't, we check the
# class dict explicitly.
cached_value = owner.__dict__.get(self._name, _UNSET)
if cached_value is _UNSET:
value = self._getter(owner)
# By setting an attribute on the class, future lookups should bypass the property entirely.
setattr(owner, self._name, value)
return value

return cached_value

def __set_name__(self, owner: _TypeT, name: str) -> None:
self._name = name


def cached_class_property(getter: Callable[[_TypeT], _T]) -> _T:
"""Similar to [functools.cached_property][], but for class properties.

Like [functools.cached_property][], the descriptor replaces itself with the computed value after the first lookup.
"""
return cast(_T, _CachedClassProperty(getter))


def reify_type_hint(attr_name: str, bound: _TypeT) -> _TypeT:
"""Creates a [cached_class_property][] which returns the type hint of the given attribute."""
return cached_class_property(lambda cls: get_mro_type_hint(cls, attr_name, bound))


def get_mro_type_hint(klass: type, attr_name: str, bound: _TypeT) -> _TypeT:
"""Returns the first type hint in `klass`'s MRO for the attribute `attr_name` and checks that it subclasses `bound`.

For unions (esp. nullable attributes), the union is checked for a member which subclasses `bound` and that is
returned. If no such member exists, a `TypeError` is raised.

Raises:
TypeError: If `klass` and its superclasses don't declare an attribute named `attr_name` or if the found type
hint and its union members don't subclass `bound`.
"""
for superclass in klass.mro():
hints = get_type_hints(superclass)
if attr_name in hints:
7 changes: 7 additions & 0 deletions questionpy/_wrappers/_qtype.py
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@

from questionpy import Question
from questionpy._wrappers._question import QuestionWrapper
from questionpy.form.validation import validate_form
from questionpy_common.api.qtype import InvalidQuestionStateError, QuestionTypeInterface
from questionpy_common.api.question import QuestionInterface
from questionpy_common.elements import OptionsFormDefinition
@@ -34,6 +35,12 @@ def __init__(
functionality. This will probably, but not necessarily, be a subclass of the default
[QuestionWrapper][questionpy.QuestionWrapper].
"""
# A form may have unresolved references when it is intended to be used as a repetition, group, section, etc.
# Only the complete form must pass validation, so the validation has to happen here instead of in FormModel or
# OptionsFormDefinition.
# We also can't do this in Question.__init_subclass__ since the type hint of options may be a forward reference.
validate_form(question_class.options_class.qpy_form)

self._question_class = question_class
self._package = package

27 changes: 27 additions & 0 deletions tests/questionpy/test_attempt.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from questionpy import Attempt, BaseAttemptState, BaseScoringState


class MyAttempt(Attempt):
formulation = ""

def _compute_score(self) -> float:
return 1

# These are forward references, i.e. the types don't exist yet. get_mro_type_hint will support this, as long as it's
# called after the types are defined, so it must be called after module execution. We test that we call
# get_type_hints correctly.
attempt_state: "MyAttemptState"
scoring_state: "MyScoringState"


class MyAttemptState(BaseAttemptState):
pass


class MyScoringState(BaseScoringState):
pass


def test_should_resolve_forward_references() -> None:
assert MyAttempt.attempt_state_class is MyAttemptState
assert MyAttempt.scoring_state_class is MyScoringState
28 changes: 28 additions & 0 deletions tests/questionpy/test_qtype.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from questionpy import BaseQuestionState, Question
from questionpy._qtype import QuestionStateWithVersion
from questionpy.form import FormModel
from tests.questionpy.test_attempt import MyAttempt


class MyQuestion(Question):
attempt_class = MyAttempt

# These are forward references, i.e. the types don't exist yet. get_mro_type_hint will support this, as long as it's
# called after the types are defined, so it must be called after module execution. We test that we call
# get_type_hints correctly.
options: "MyFormModel"
question_state: "MyQuestionState"


class MyFormModel(FormModel):
pass


class MyQuestionState(BaseQuestionState):
pass


def test_should_resolve_forward_references() -> None:
assert MyQuestion.options_class is MyFormModel
assert MyQuestion.question_state_class is MyQuestionState
assert MyQuestion.question_state_with_version_class is QuestionStateWithVersion[MyFormModel, MyQuestionState]
19 changes: 18 additions & 1 deletion tests/questionpy/wrappers/test_qtype.py
Original file line number Diff line number Diff line change
@@ -5,14 +5,17 @@

import pytest

from questionpy import OptionsFormValidationError, QuestionTypeWrapper, QuestionWrapper
from questionpy import OptionsFormValidationError, Question, QuestionTypeWrapper, QuestionWrapper
from questionpy.form import FormModel, is_not_checked, text_input
from questionpy.form.validation import FormError
from questionpy_common.elements import OptionsFormDefinition, TextInputElement
from questionpy_common.environment import Package
from tests.questionpy.wrappers.conftest import (
QUESTION_STATE_DICT,
STATIC_FILES,
QuestionUsingDefaultState,
QuestionUsingMyQuestionState,
SomeAttempt,
)

_EXPECTED_FORM = OptionsFormDefinition(general=[TextInputElement(name="input", label="Some Label")])
@@ -72,3 +75,17 @@ def test_should_preserve_options_when_using_default_question_state(package: Pack

def test_should_get_static_files(package: Package) -> None:
package.manifest.static_files = STATIC_FILES


def test_should_raise_when_form_is_invalid(package: Package) -> None:
class ModelWithUnresolvedReference(FormModel):
my_text: str | None = text_input("Label", hide_if=is_not_checked("nonexistent_checkbox"))

class MyQuestion(Question):
attempt_class = SomeAttempt
options: ModelWithUnresolvedReference

# There are more detailed validation tests in test_validation.py, here we just ensure that validate_form is called
# at the correct place.
with pytest.raises(FormError):
QuestionTypeWrapper(MyQuestion, package)
16 changes: 0 additions & 16 deletions tests/questionpy/wrappers/test_question.py
Original file line number Diff line number Diff line change
@@ -9,13 +9,10 @@
from questionpy import (
InvalidResponseError,
NeedsManualScoringError,
Question,
QuestionTypeWrapper,
QuestionWrapper,
ResponseNotScorableError,
)
from questionpy.form import FormModel, is_not_checked, text_input
from questionpy.form.validation import FormError
from questionpy_common.api.attempt import AttemptModel, AttemptScoredModel, AttemptStartedModel, AttemptUi, ScoringCode
from questionpy_common.api.question import QuestionModel, ScoringMethod
from questionpy_common.environment import Package
@@ -106,16 +103,3 @@ def test_should_export_question_model(package: Package) -> None:
question_model = question.export()

assert question_model == QuestionModel(lang="en", scoring_method=ScoringMethod.AUTOMATICALLY_SCORABLE)


def test_should_raise_when_form_is_invalid() -> None:
class ModelWithUnresolvedReference(FormModel):
my_text: str | None = text_input("Label", hide_if=is_not_checked("nonexistent_checkbox"))

# There are more detailed validation tests in test_validation.py, here we just ensure that validate_form is called
# at the correct place.
with pytest.raises(FormError):

class MyQuestion(Question):
attempt_class = SomeAttempt
options: ModelWithUnresolvedReference