Skip to content

Fix Quality and Docs tests for Problem XBlock #153

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.

Sign up for GitHub

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

Open
irtazaakram wants to merge 1 commit into
base: fix-problemblock
Choose a base branch
from
Open

Fix Quality and Docs tests for Problem XBlock #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions xblocks_contrib/html/html.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def get_context(self):
# the root /c4x/ url for assets. This allows client-side substitutions to occur.
return {
"module": self,
"editable_metadata_fields": self.editable_metadata_fields, # pylint: disable=no-member
"editable_metadata_fields": self.editable_metadata_fields,
"data": self.data,
"base_asset_url": self.get_base_url_path_for_course_assets(self.location.course_key),
"enable_latex_compiler": self.use_latex_compiler,
Expand Down Expand Up @@ -371,7 +371,7 @@ def bind_for_student(self, user_id, wrappers=None):
if self.scope_ids.user_id is not None and user_id == self.scope_ids.user_id:
if getattr(self.runtime, "position", None):
# update the position of the tab
self.position = self.runtime.position # pylint: disable=attribute-defined-outside-init
self.position = self.runtime.position
return

# # If we are switching users mid-request, save the data from the old user.
Expand Down Expand Up @@ -402,7 +402,7 @@ def bind_for_student(self, user_id, wrappers=None):
wrapped_field_data = self.runtime.service(self, "field-data-unbound")
for wrapper in wrappers:
wrapped_field_data = wrapper(wrapped_field_data)
self._bound_field_data = wrapped_field_data # pylint: disable=attribute-defined-outside-init
self._bound_field_data = wrapped_field_data
if getattr(self.runtime, "uses_deprecated_field_data", False):
# This approach is deprecated but old mongo's CachingDescriptorSystem still requires it.
# For Split mongo's CachingDescriptor system, don't set ._field_data this way.
Expand Down
43 changes: 28 additions & 15 deletions xblocks_contrib/problem/capa/capa_problem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
# main class for this module


class LoncapaSystem: # pylint: disable=too-few-public-methods,too-many-instance-attributes
class LoncapaSystem:
"""
An encapsulation of resources needed from the outside.

Expand Down Expand Up @@ -127,7 +127,7 @@ def __init__( # pylint: disable=too-many-positional-arguments,too-many-argument
self.matlab_api_key = matlab_api_key


class LoncapaProblem: # pylint: disable=too-many-public-methods,too-many-instance-attributes
class LoncapaProblem:
"""
Main class for capa Problems.
"""
Expand Down Expand Up @@ -239,7 +239,7 @@ def __init__( # pylint: disable=too-many-positional-arguments,too-many-argument
# Run response late_transforms last (see MultipleChoiceResponse)
# Sort the responses to be in *_1 *_2 ... order.
responses = list(self.responders.values())
responses = sorted(responses, key=lambda resp: int(resp.id[resp.id.rindex("_") + 1 :]))
responses = sorted(responses, key=lambda resp: int(resp.id[resp.id.rindex("_") + 1:]))
for response in responses:
if hasattr(response, "late_transforms"):
response.late_transforms(self)
Expand All @@ -251,21 +251,35 @@ def make_xml_compatible(self, tree):
"""
Adjust tree xml in-place for compatibility before creating
a problem from it.

The idea here is to provide a central point for XML translation,
for example, supporting an old XML format. At present, there just two translations.

1. <additional_answer> compatibility translation:
old: <additional_answer>ANSWER</additional_answer>
convert to
new: <additional_answer answer="ANSWER">OPTIONAL-HINT</addional_answer>

old::

<additional_answer>ANSWER</additional_answer>

convert to

new::

<additional_answer answer="ANSWER">OPTIONAL-HINT</addional_answer>

2. <optioninput> compatibility translation:
optioninput works like this internally:
<optioninput options="('yellow','blue','green')" correct="blue" />
With extended hints there is a new <option> tag, like this
<option correct="True">blue <optionhint>sky color</optionhint> </option>
This translation takes in the new format and synthesizes the old option= attribute
so all downstream logic works unchanged with the new <option> tag format.

optioninput works like this internally::

<optioninput options="('yellow','blue','green')" correct="blue" />

With extended hints there is a new <option> tag, like this::

<option correct="True">blue <optionhint>sky color</optionhint></option>

This translation takes in the new format and synthesizes the old
``option=`` attribute so all downstream logic works unchanged with
the new <option> tag format.
"""

def is_optioninput_valid(optioninput):
Expand Down Expand Up @@ -362,8 +376,7 @@ def get_max_score(self):
def calculate_score(self, correct_map=None):
"""
Compute score for this problem. The score is the number of points awarded.
Returns a dictionary {'score': integer, from 0 to get_max_score(),
'total': get_max_score()}.
Returns a dictionary {'score': integer, from 0 to get_max_score(), 'total': get_max_score()}.

Takes an optional correctness map for use in the rescore workflow.
"""
Expand Down Expand Up @@ -957,7 +970,7 @@ def _extract_context(self, tree):

def _extract_html( # private
self, problemtree
): # pylint: disable=too-many-statements,too-many-locals,too-many-branches,too-many-return-statements
): # pylint: disable=too-many-locals,too-many-branches,too-many-return-statements
"""
Main (private) function which converts Problem XML tree to HTML.
Calls itself recursively.
Expand Down
7 changes: 5 additions & 2 deletions xblocks_contrib/problem/capa/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
log = logging.getLogger("capa.checker")


class DemoSystem: # pylint: disable=too-few-public-methods
class DemoSystem:
"""Render templates using Django's template engine."""

def __init__(self):
Expand Down Expand Up @@ -114,12 +114,15 @@ def check_that_blanks_fail(problem):


def check_that_suggested_answers_work(problem):
"""Split this up so that we're only used for formula/numeric answers.
"""
Split this up so that we're only used for formula/numeric answers.

Examples of where this fails:

* Displayed answers use units but acceptable ones do not.
- L1e0.xml
- Presents itself as UndefinedVariable (when it tries to pass to calc)

* "a or d" is what's displayed, but only "a" or "d" is accepted, not the
string "a or d".
- L1-e00.xml
Expand Down
12 changes: 6 additions & 6 deletions xblocks_contrib/problem/capa/customrender.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
# -----------------------------------------------------------------------------


class MathRenderer: # pylint: disable=too-few-public-methods
class MathRenderer:
"""
Renders <math> tags into MathJax-compatible HTML for displaying math expressions.
"""
Expand Down Expand Up @@ -63,7 +63,7 @@ def get_html(self):
html = f"<html><html>{self.mathstr}</html><html>{saxutils.escape(self.xml.tail or '')}</html></html>"
try:
xhtml = etree.XML(html)
except Exception as err: # pylint: disable=broad-exception-caught
except Exception as err:
if self.system.DEBUG:
msg = (
f"<html><div class='inline-error'>"
Expand All @@ -85,7 +85,7 @@ def get_html(self):
# -----------------------------------------------------------------------------


class SolutionRenderer: # pylint: disable=too-few-public-methods
class SolutionRenderer:
"""
A solution is just a <span>...</span> which is given an ID, that is used for displaying an
extended answer (a problem "solution") after "show answers" is pressed.
Expand Down Expand Up @@ -113,7 +113,7 @@ def get_html(self):
# -----------------------------------------------------------------------------


class TargetedFeedbackRenderer: # pylint: disable=too-few-public-methods
class TargetedFeedbackRenderer:
"""
A targeted feedback is just a <span>...</span> that is used for displaying an
extended piece of feedback to students if they incorrectly answered a question.
Expand All @@ -138,7 +138,7 @@ def get_html(self):
try:
xhtml = etree.XML(html_str)

except Exception as err: # pylint: disable=broad-exception-caught
except Exception as err:
if self.system.DEBUG:
# xss-lint: disable=python-wrap-html
msg = f"""
Expand All @@ -163,7 +163,7 @@ def get_html(self):
# -----------------------------------------------------------------------------


class ClarificationRenderer: # pylint: disable=too-few-public-methods
class ClarificationRenderer:
"""
A clarification appears as an inline icon which reveals more information when the user
hovers over it.
Expand Down
Loading