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

Detect blanks with exercise.blanks opt, and add parse checking #547

Merged
merged 67 commits into from
Aug 24, 2021

Conversation

rossellhayes
Copy link
Contributor

@rossellhayes rossellhayes commented Jun 25, 2021

This PR modifies evaluate_exercise() to check whether submitted code contains blanks and whether submitted code cannot be parsed.

If submitted code is unparsable, the R error message is rendered along with a message copied from gradethis explaining that the code could not be parsed and hinting at common causes of this problem.

One special case of unparsability is code that contains blanks. Often, tutorial authors will include blanks in their prompts or hints for students to fill in with valid code, e.g. ___(___, na.rm = TRUE). If submitted code contains blanks, evaluation proceeds as normal, but any feedback is replaced by a message explaining that the submitted code contains blanks and showing what blanks in the exercise look like. This applies even if the code containing blanks is parsable, e.g. if the blank is within a character string. To achieve this, the function return_if_exercise_result() is added within evaluate_exercise(). Each existing check (result check, code check, and error check) is wrapped within that function. This function initiates an early return if a result is signalled, but replaces the feedback with feedback about blanks if blanks are present in the user's code.

By default, any string of three or more underscores is considered to be a blank. This is the most common pattern in existing tutorials. But this definition can be changed with the exercise.blanks chunk option, which accepts a regex pattern to detect as blanks. For example, using {r exercise-check, exercise.blanks = "\\.{3,}"} as a chunk header would detect series of three or more dots as blanks.

Both unparsable and blanks messages are implemented with i18next keys, allowing them to be translated into the same language as the rest of the learnr interface. Instead of producing raw text when encountering unparsable code or code containing blanks, R generates a span tag with data-i18n attributes that are replaced with the correct localized text by the i18next JavaScript library on render. This PR will necessitate adding translations for blank, blank_plural, exercisecontainsblank, pleasereplaceblank, unparsable, and, and or.

@rossellhayes rossellhayes changed the title Add handler for underscores to render_exercise() Add handler for unparsable user code Jun 27, 2021
@rossellhayes rossellhayes marked this pull request as ready for review June 27, 2021 03:33
Copy link
Member

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

This looks great, thank you!

One design question: In gradethis we give users an (undocumented) option as a way to turn this off or modify how unparseable code is handled. Should we also do that in learnr? Perhaps an option analogous to exercise.error.check.code, e.g. exercise.error.parse.check. (There may already be a better option in use in learnr.)

tests/testthat/test-exercise.R Outdated Show resolved Hide resolved
R/exercise.R Outdated Show resolved Hide resolved
R/exercise.R Outdated Show resolved Hide resolved
@rossellhayes
Copy link
Contributor Author

rossellhayes commented Jun 29, 2021

Are you thinking an exercise.parse.error.check would be implemented with global options() or with chunk options? I've implemented it as a chunk option for now.

rossellhayes and others added 4 commits June 29, 2021 12:24
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
@gadenbuie
Copy link
Member

  1. Always run parse checker on submitted code before any evaluation
  2. Follow the pattern of the error-check exercise checker
    • Use the exercise.checker function
    • Use the code from *-parse-check chunk as the check_code for that function
    • Or fall back to the global option exercise.parse.check.code
  3. If parse failure, return the parse error message with additional friendly feedback

@rossellhayes
Copy link
Contributor Author

rossellhayes commented Jul 2, 2021

Running into a few test failures on old releases because the new code uses isFALSE(), which wasn't available on R < 3.5.
I could:

  1. use backports,
  2. include isFALSE() as an internal function, or
  3. refactor.

For now I've included it as an internal function in utils.R.

@rossellhayes
Copy link
Contributor Author

rossellhayes commented Jul 6, 2021

Here's the current state of the checking:

  1. Evaluate the global setup chunk.
  2. Blank checking can be specified with exercise.blank.check.code. If it's FALSE, no blank checking is performed. If it's NULL, the default exercise_blank_checker() is called. Otherwise, it can be a function.
  3. The default blank check is an exercise.checker that marks the exercise incorrect and tells the user how many blanks are still in their code. It detects both parsable (e.g. in strings) and unparsable blanks.
  4. Blanks are specified with a regex in the exercise.blanks option. This defaults to "_{3,}".
  5. If there are no blanks, move to parse checking.
  6. If the exercise engine is not R, skip parse checking.
  7. Parse checking can be specified with exercise.blank.check.code, using the same process as step 2.
  8. The default parse check is an exercise.checker that marks the exercise as incorrect and an error if the user code cannot be parsed. It gives the same message as gradethis (you may have missed a blank, comma, end parenthesis, etc).
  9. If the code parses, then the existing process begins: creating the exercise directory, code checking, etc.

@rossellhayes
Copy link
Contributor Author

These changes look good to me!

R/exercise.R Outdated Show resolved Hide resolved
R/exercise.R Outdated Show resolved Hide resolved
Return nothing to better signal this function is called for side-effects, not return values
When the `oxford_comma` argument was added to `knitr::combine_words()`
@gadenbuie gadenbuie self-requested a review August 24, 2021 14:13
@gadenbuie gadenbuie changed the title Add handler for unparsable user code Detect blanks with exercise.blanks opt, and add parse checking Aug 24, 2021
@gadenbuie gadenbuie merged commit dc5ef9e into rstudio:master Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants