-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Anton/allow arbitrary regex in stringresponse #1948
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -329,8 +329,8 @@ def get_hints(self, student_answers, new_cmap, old_cmap): | |
| rephints = hintgroup.findall(self.hint_tag) | ||
| hints_to_show = self.check_hint_condition( | ||
| rephints, student_answers) | ||
|
|
||
| # can be 'on_request' or 'always' (default) | ||
|
|
||
| hintmode = hintgroup.get('mode', 'always') | ||
| for hintpart in hintgroup.findall('hintpart'): | ||
| if hintpart.get('on') in hints_to_show: | ||
|
|
@@ -947,55 +947,128 @@ def get_answers(self): | |
|
|
||
| class StringResponse(LoncapaResponse): | ||
| ''' | ||
| This response type allows one or more answers. Use `_or_` separator to set | ||
| more than 1 answer. | ||
| This response type allows one or more answers. | ||
|
|
||
| Example: | ||
| Additional answers are added by `additional_answer` tag. | ||
| If `regexp` is in `type` attribute, than answers and hints are treated as regular expressions. | ||
|
|
||
| # One answer | ||
| Examples: | ||
| <stringresponse answer="Michigan"> | ||
| <textline size="20" /> | ||
| </stringresponse > | ||
|
|
||
| # Multiple answers | ||
| <stringresponse answer="Martin Luther King_or_Dr. Martin Luther King Jr."> | ||
| <textline size="20" /> | ||
| <textline size="20" /> | ||
| </stringresponse > | ||
|
|
||
| <stringresponse answer="a1" type="ci regexp"> | ||
| <additional_answer>\d5</additional_answer> | ||
| <additional_answer>a3</additional_answer> | ||
| <textline size="20"/> | ||
| <hintgroup> | ||
| <stringhint answer="a0" type="ci" name="ha0" /> | ||
| <stringhint answer="a4" type="ci" name="ha4" /> | ||
| <stringhint answer="^\d" type="ci" name="re1" /> | ||
| <hintpart on="ha0"> | ||
| <startouttext />+1<endouttext /> | ||
| </hintpart > | ||
| <hintpart on="ha4"> | ||
| <startouttext />-1<endouttext /> | ||
| </hintpart > | ||
| <hintpart on="re1"> | ||
| <startouttext />Any number+5<endouttext /> | ||
| </hintpart > | ||
| </hintgroup> | ||
| </stringresponse> | ||
| ''' | ||
| response_tag = 'stringresponse' | ||
| hint_tag = 'stringhint' | ||
| allowed_inputfields = ['textline'] | ||
| required_attributes = ['answer'] | ||
| max_inputfields = 1 | ||
| correct_answer = [] | ||
| SEPARATOR = '_or_' | ||
|
|
||
| def setup_response_backward(self): | ||
| self.correct_answer = [ | ||
| contextualize_text(answer, self.context).strip() for answer in self.xml.get('answer').split('_or_') | ||
| ] | ||
|
|
||
| def setup_response(self): | ||
| self.correct_answer = [contextualize_text(answer, self.context).strip() | ||
| for answer in self.xml.get('answer').split(self.SEPARATOR)] | ||
|
|
||
| self.backward = '_or_' in self.xml.get('answer').lower() | ||
| self.regexp = 'regexp' in self.xml.get('type').lower().split(' ') | ||
| self.case_insensitive = 'ci' in self.xml.get('type').lower().split(' ') | ||
|
|
||
| # backward compatibility, can be removed in future, it is up to @Lyla Fisher. | ||
| if self.backward: | ||
| self.setup_response_backward() | ||
| return | ||
| # end of backward compatibility | ||
|
|
||
| correct_answers = [self.xml.get('answer')] + [el.text for el in self.xml.findall('additional_answer')] | ||
| self.correct_answer = [contextualize_text(answer, self.context).strip() for answer in correct_answers] | ||
|
|
||
| # remove additional_answer from xml, otherwise they will be displayed | ||
| for el in self.xml.findall('additional_answer'): | ||
| self.xml.remove(el) | ||
|
|
||
| def get_score(self, student_answers): | ||
| '''Grade a string response ''' | ||
| student_answer = student_answers[self.answer_id].strip() | ||
| correct = self.check_string(self.correct_answer, student_answer) | ||
| return CorrectMap(self.answer_id, 'correct' if correct else 'incorrect') | ||
|
|
||
| def check_string(self, expected, given): | ||
| if self.xml.get('type') == 'ci': | ||
| def check_string_backward(self, expected, given): | ||
| if self.case_insensitive: | ||
| return given.lower() in [i.lower() for i in expected] | ||
| return given in expected | ||
|
|
||
| def check_string(self, expected, given): | ||
| """ | ||
| Find given in expected. | ||
|
|
||
| If self.regexp is true, regular expression search is used. | ||
| if self.case_insensitive is true, case insensitive search is used, otherwise case sensitive search is used. | ||
| Spaces around values of attributes are stripped in XML parsing step. | ||
|
|
||
| Args: | ||
| expected: list. | ||
| given: str. | ||
|
|
||
| Returns: bool | ||
|
|
||
| Raises: `ResponseError` if it fails to compile regular expression. | ||
|
|
||
| Note: for old code, which supports _or_ separator, we add some backward compatibility handling. | ||
| Should be removed soon. When to remove it, is up to Lyla Fisher. | ||
| """ | ||
| # backward compatibility, should be removed in future. | ||
| if self.backward: | ||
| return self.check_string_backward(expected, given) | ||
| # end of backward compatibility | ||
|
|
||
| if self.regexp: # regexp match | ||
| flags = re.IGNORECASE if self.case_insensitive else 0 | ||
| try: | ||
| regexp = re.compile('^'+ '|'.join(expected) + '$', flags=flags | re.UNICODE) | ||
| result = re.search(regexp, given) | ||
| except Exception as err: | ||
| msg = '[courseware.capa.responsetypes.stringresponse] error: {}'.format(err.message) | ||
| log.error(msg, exc_info=True) | ||
| raise ResponseError(msg) | ||
| return bool(result) | ||
| else: # string match | ||
| if self.case_insensitive: | ||
| return given.lower() in [i.lower() for i in expected] | ||
| else: | ||
| return given in expected | ||
|
|
||
|
|
||
| def check_hint_condition(self, hxml_set, student_answers): | ||
| given = student_answers[self.answer_id].strip() | ||
| hints_to_show = [] | ||
| for hxml in hxml_set: | ||
| name = hxml.get('name') | ||
|
|
||
| correct_answer = [contextualize_text(answer, self.context).strip() | ||
| for answer in hxml.get('answer').split(self.SEPARATOR)] | ||
| hinted_answer = contextualize_text(hxml.get('answer'), self.context).strip() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you need to deal with backward compatibility here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, because hxml is xml for hints and it is different from problem xml.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the old code split on self.SEPARATOR. Was it unneeded?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some thoughts why we do not need to support that in hints:
|
||
|
|
||
| if self.check_string(correct_answer, given): | ||
| if self.check_string([hinted_answer], given): | ||
| hints_to_show.append(name) | ||
| log.debug('hints_to_show = %s', hints_to_show) | ||
| return hints_to_show | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these simple examples no longer right? It would be good to keep them in the docstring if they still work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed: restored correct simple example.