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

Better handling of infix operators #84

Open
chendaniely opened this issue Oct 10, 2019 · 4 comments · May be fixed by #332
Open

Better handling of infix operators #84

chendaniely opened this issue Oct 10, 2019 · 4 comments · May be fixed by #332
Labels
grade code Related to automatic code grading

Comments

@chendaniely
Copy link
Member

given:

  user <-     quote(sqrt(1))
  solution <- quote(sqrt(1 + 2))
  expect_equal(
    detect_mistakes(user, solution)
    ,
    wrong_value(this = quote(1), that = "1 + 2")
  )

returns

Error: detect_mistakes(user, solution) not equal to wrong_value(this = quote(1), that = "1 + 2").
1/1 mismatches
x[1]: "I expected a call to `+`() where it was interpreted as 1."
y[1]: "I expected 1 + 2 where it was interpreted as 1."
@garrettgman
Copy link
Member

Another error with +:

Screen Shot 2020-03-11 at 4 06 14 PM

garrettgman added a commit that referenced this issue Mar 16, 2020
garrettgman added a commit that referenced this issue Mar 16, 2020
garrettgman added a commit that referenced this issue Mar 31, 2020
* First pass at revised detect_mistakes - solves initial + inquiry.

* Debugs detect_mistake to pass all tests, adds test for + case, edits detect_mistakes to flagged unmatched named arguments passed to ... as surplus

* Adds tests for #84, #91, #94, #95.

* Cleans up tests in test_detect_mistakes.R. Tests no longer use name of first argument.

* Checks user code for bad syntax before standardising. Fixes #91, #94, #95.

* Adds message generators for new cases: wrong_call(), bad_argument_name(), too_many_arguments().

* Adds new message generators to detect_mistakes.

* Adds enclosing calls to student messages

* Adds sanity-check.Rmd for checking that messages are sensible (which is not easy to automate).

* Streamlines message generators.

* Fixes bug in detect_mistakes that failed to catch unused unnamed arguments before passing student code to call_standardise_formals.

* Improves the messages returned by the message generators.

* Completes suite of sanity_checks for reading whether or not gradethis provides intelligible messages to students.

* Corrects argument names in too_many_matches

* Adds checks in sanity_checks.Rmd to tests as test_sanity_checks.R

* Moves sanity-checks.Rmd to the tutorials as grade_code-messages

* Changes content of old sanity-checks.R to be a tutorial.

* Fixes awkward message returned by missing_argument when the missing argument does not have a name.

* Fixes hard-coded tests that incorrectly fail in the presence of improvements.

* Adds your_char variable to missing_argument() based on reviewer feedback

* Improves handling of comparisons in build_intro() based on reviewer feedback.

* Checks for case where user uses the same argument name twice. Adds duplicate_name() message generator.

* Small improvements based on reviewer feedback

* Fixes bug in how detect_mistakes handles partially matched formal names.

* Further refactoring with real_names() inside of detect_mistakes()

* Fixes bug that caused detect_mistakes to prematurely return  NULL (correct) is one set of non-identical arguments were none-the-less correct.
@gadenbuie
Copy link
Member

Here's a reprex as this stands now. @garrettgman do you have any suggestions for the messages we wish to see?

pkgload::load_all("~/work/gradethis")
#> Loading gradethis (a8f772102d5b51c2ff3810d45fb4f532f94d8a72)

code_feedback("sqrt(1)", "sqrt(1 + 2)")
#> In `sqrt(1)`, I expected `+` where you wrote `1`.

code_feedback("0 + sqrt(log(2))", "sqrt(log(2))")
#> I expected you to call `sqrt()` where you called `+`.

@gadenbuie gadenbuie added the grade code Related to automatic code grading label Jan 11, 2021
@garrettgman
Copy link
Member

For infix operators , I think we need to say "something + something" instead of +. For example, the feedback message below

Screen Shot 2021-04-12 at 4 48 25 PM

Suggests to a new user that they should do this, which leads them to a frustrating dead end that they may not be able to see their way out of:

Screen Shot 2021-04-12 at 4 48 48 PM

A better message would be "In flipper_length_mm > bill_length_mm, I expected something * something where you wrote bill_length_mm." Alternatively, we could replace + with "a call to +()" in our messages, but new R users will have trouble interpreting that.

This replacement should generalize to all infix operators.

Should I open this as a separate issue? Its not clear to me what the thrust of Daniel's issue was.

@gadenbuie
Copy link
Member

Here's another example.

```{r starwars-setup}
library(dplyr)
```

```{r starwars, exercise=TRUE}
starwars %>%
  transmute(
    mass = as.integer(mass * 2.205),
    height = as.integer(height / 2.54)
  )
```

```{r starwars-solution}
starwars %>%
  transmute(
    height = height / 2.54, 
    mass = mass * 2.205
  )
```

```{r starwars-check}
grade_this_table(check_column_order = TRUE)
```

Your table’s columns were not in the expected order. The first 2 columns of your table should be height and mass.

I expected you to call height = / where you called height = as.integer(). Give it another try.

@rossellhayes rossellhayes linked a pull request Feb 16, 2023 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
grade code Related to automatic code grading
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants