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

Warning when exercise gives invisible result #369

Closed
dtkaplan opened this issue May 14, 2020 · 2 comments · Fixed by #373
Closed

Warning when exercise gives invisible result #369

dtkaplan opened this issue May 14, 2020 · 2 comments · Fixed by #373

Comments

@dtkaplan
Copy link
Collaborator

@dtkaplan dtkaplan commented May 14, 2020

There's an internal function in exercise.R that is called when the exercise code produces an invisible result. I'm not sure why an invisible result is something you want to check for and why this feature is needed at all. @garrettgman

I think the warning being displayed will be unfriendly to most students. Might we either

  1. Allow it to be configured/disabled as an option.
  2. Set it to something more friendly, like: "Note: Using assignment in the last line means that no value will be printed." Of course, you also get the warning when the last line generates a side effect but no value, e.g. plot(1:10)
  3. Leave it to the code checking. If last_value is NULL, that might suggest a helpful hint depending on the context, but only the author of the tutorial can decide what that should be.
invisible_feedback <- function() {
  feedback_as_html(
    feedback_validated(
      list(
        message = "Last value being used to check answer is invisible. See `?invisible` for more information",
        type = "warning",
        correct = FALSE,
        location = "append"
      )
    )
  )
}
@mine-cetinkaya-rundel
Copy link
Collaborator

@mine-cetinkaya-rundel mine-cetinkaya-rundel commented May 20, 2020

I second this suggestion! I like options 1 and/or 3. I think a combination of the two makes sense.

@garrettgman
Copy link
Member

@garrettgman garrettgman commented May 20, 2020

I agree with @dtkaplan and @mine-cetinkaya-rundel. learnr probably doesn't intend to tell the student that their answer is invalid for learnr.

I vote for option 3. Since this is a check, we should leave it to the author and the checking package.

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 a pull request may close this issue.

3 participants