swirldev / swirl

:cyclone: Learn R, in R.
http://swirlstats.com
Other
1.11k stars 590 forks source link

Doesn't detect 'cars' properly (can't make progress past it) #116

Closed chr0nikler closed 10 years ago

chr0nikler commented 10 years ago

Simply put, at this question in the course Data Analsysis Central Tendency:

| To illustrate these concepts, we will now look at a real dataset from the 'openintro' R package, which has already | been loaded for you. Type 'cars' and press Enter to see the dataset we'll be working with.

I type in cars, hit enter, and it tells me "Not quite" or "you almost had it" even though I am typing in exactly what they ask for. Others seem to have progressed passed this, but I don't see why I can't. The data is there, since I can see it. swirl just won't let me past this question.

Further Details: Ubuntu 12.04 sessionInfo: rsessioninfo

ncarchedi commented 10 years ago

Please include either a screenshot or copy/paste of exactly what swirl asked, what you entered, and what the output was. Can you also clear your workspace with rm(list=ls()) then restart R to see if that helps.

chr0nikler commented 10 years ago

Since it is too big for a screenshot, here is a copy/paste:

| To illustrate these concepts, we will now look at a real dataset from the 'openintro' R package, which has already | been loaded for you. Type 'cars' and press Enter to see the dataset we'll be working with.

cars type price mpgCity driveTrain passengers weight 1 small 15.9 25 front 5 2705 2 midsize 33.9 18 front 5 3560 3 midsize 37.7 19 front 6 3405 4 midsize 30.0 22 rear 4 3640 5 midsize 15.7 22 front 6 2880 6 large 20.8 19 front 6 3470 7 large 23.7 16 rear 6 4105 8 midsize 26.3 19 front 5 3495 9 large 34.7 16 front 6 3620 10 midsize 40.1 16 front 5 3935 11 midsize 15.9 21 front 6 3195 12 large 18.8 17 rear 6 3910 13 large 18.4 20 front 6 3515 14 large 29.5 20 front 6 3570 15 small 9.2 29 front 5 2270 16 small 11.3 23 front 5 2670 17 midsize 15.6 21 front 6 3080 18 small 12.2 29 front 5 2295 19 large 19.3 20 front 6 3490 20 small 7.4 31 front 4 1845 21 small 10.1 23 front 5 2530 22 midsize 20.2 21 front 5 3325 23 large 20.9 18 rear 6 3950 24 small 8.4 46 front 4 1695 25 small 12.1 42 front 4 2350 26 small 8.0 29 front 5 2345 27 small 10.0 22 front 5 2620 28 midsize 13.9 20 front 5 2885 29 midsize 47.9 17 rear 5 4000 30 midsize 28.0 18 front 5 3510 31 midsize 35.2 18 rear 4 3515 32 midsize 34.3 17 front 6 3695 33 large 36.1 18 rear 6 4055 34 small 8.3 29 front 4 2325 35 small 11.6 28 front 5 2440 36 midsize 61.9 19 rear 5 3525 37 midsize 14.9 19 rear 5 3610 38 small 10.3 29 front 5 2295 39 midsize 26.1 18 front 5 3730 40 small 11.8 29 front 5 2545 41 midsize 21.5 21 front 5 3200 42 midsize 16.3 23 front 5 2890 43 large 20.7 19 front 6 3470 44 small 9.0 31 front 4 2350 45 midsize 18.5 19 front 5 3450 46 large 24.4 19 front 6 3495 47 small 11.1 28 front 5 2495 48 small 8.4 33 4WD 4 2045 49 small 10.9 25 4WD 5 2490 50 small 8.6 39 front 4 1965 51 small 9.8 32 front 5 2055 52 midsize 18.2 22 front 5 3030 53 small 9.1 25 front 4 2240 54 midsize 26.7 20 front 5 3245

| Not exactly. Give it another go. Or, type info() for more options.

| Type 'cars' and press Enter. Do not use quotes, spaces, or uppercase letters.

I did the rm(list=lis()), restarted R, then went back into swirl. Same issue.

WilCrofter commented 10 years ago

This looks like a bug. Debugging Data Analysis, Central Tendency, question 17, leads to the test the following line of code (answerTests.R line 385) which fails.

results <- swirlExpectation(condition(object))

Tracking this down leads to testthat:::compare(expected, actual) which fails inexplicably. The variables expected and actual are the same expressions as far as I can tell. Either this is a bug in testthat or a current version of testthat is incompatible with the one under which Data Analysis was developed.

I'm stumped for an easy fix at this point. Perhaps we shall have to revise the entire course.

WilCrofter commented 10 years ago

This bug or incompatibility will probably affect all the early courses, the ones which use AnswerTests.R rather than AnswerTests2.R. It is surely easier to rewrite AnswerTests.R than to rewrite all the courses. We can test the idea using a custom test file to override the keyphrase-style tests. That way we needn't involve CRAN and can fix the affected courses by adding customTest.R files to them. There's no hope of doing it this week, with swirl in use at Coursera, but I imagine we could get to it in the next week or two.

ncarchedi commented 10 years ago

@WilCrofter Are you able to reproduce the bug on your machine?

WilCrofter commented 10 years ago

On Wed, 09 Apr 2014 05:15:27 -0700 Nick Carchedi notifications@github.com wrote:

@WilCrofter Are you able to reproduce the bug on your machine?


Reply to this email directly or view it on GitHub: https://github.com/swirldev/swirl/issues/116#issuecomment-39956698

Yes. It looks like a testthat bug or incompatibility which will require us to rewrite the old answer tests. I've posted details on the issue page.

WilCrofter commented 10 years ago

It's an R Version issue, having to do with 3.1.0 beta and possibly the tools package. Notice the bug occurs in both x86_64-pc-linux-gnu below, and in Joraaver's x86_32-pc-linux-gnu above.

Session in which bug occurs:

R version 3.1.0 beta (2014-03-28 r65330) -- "Spring Dance" Copyright (C) 2014 The R Foundation for Statistical Computing Platform: x86_64-pc-linux-gnu (64-bit)

other attached packages: [1] testthat_0.8.1 openintro_1.4 plotrix_3.5-5 swirl_2.2.0

loaded via a namespace (and not attached): [1] digest_0.6.4 httr_0.3 RCurl_1.95-4.1 stringr_0.6.2 tools_3.1.0 yaml_2.1.11

Session in which bug does not occur

R version 3.0.3 (2014-03-06) -- "Warm Puppy" Copyright (C) 2014 The R Foundation for Statistical Computing Platform: i686-pc-linux-gnu (32-bit)

other attached packages: [1] testthat_0.8.1 openintro_1.4 plotrix_3.5-5 swirl_2.2.0

loaded via a namespace (and not attached): [1] digest_0.6.4 httr_0.3 RCurl_1.95-4.1 stringr_0.6.2 tools_3.0.3
[6] yaml_2.1.11

WilCrofter commented 10 years ago

R 3.1.0 release (not beta) does not solve the problem. Does it occur on operating systems other than ubuntu and its derivatives?

WilCrofter commented 10 years ago

The failure to detect cars occurs in this function:

# Returns TRUE if as.expression
# (e$expr) matches the expression indicated to the right
# of "=" in keyphrase
# keyphrase:equivalent=expression
runTest.equivalent <- function(keyphrase,e) {
  correctExpr <- parse(text=rightside(keyphrase))
  userExpr <- as.expression(e$expr)
  results <- expectThat(userExpr,
                        is_equivalent_to(correctExpr,deparse(correctExpr)),
                        label=deparse(userExpr))

  if(is(e,"dev") && !results$passed)swirl_out(results$message) 
  return(results$passed) ## BREAKPOINT HERE
}

The test fails, since results$passed, which was TRUE prior to R 3.1.0, is now FALSE.

Browse[2]> results
Not expected: not equal to expected
target is not list-like. 
Browse[2]> results$passed
FALSE

Why? Note that correctExpr is an expression of length 1 containing the symbol cars, and with attributes added by the parse function. Since is_equivalent_to ignores attributes, these should make no difference.

Browse[2]> str(correctExpr)
length 1 expression(cars)
 - attr(*, "srcref")=List of 1
  ..$ :Class 'srcref'  atomic [1:8] 1 1 1 4 1 4 1 1
  .. .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x4d691e0> 
 - attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x4d691e0> 
 - attr(*, "wholeSrcref")=Class 'srcref'  atomic [1:8] 1 0 2 0 0 0 1 2
  .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x4d691e0> 

The target, userExpr, is an expression not a length 1 expression. Presumably this is what is meant by "target is not list-like"

Browse[2]> str(userExpr)
  expression(cars)

The target is list-like however, since we can subset it as a list. And, in fact,

Browse[2]> is_equivalent_to(correctExpr[[1]])(userExpr[[1]])
As expected: equals expected 

And

Browse[2]> is_equivalent_to(as.list(correctExpr))(as.list(userExpr))
As expected: equals expected

So we've run into an R version incompatibility. I'm working on a fix in a branch. I want it to be backwards compatible, so it may take a while. Stay tuned.

WilCrofter commented 10 years ago

A cursory look suggests that runTest.equivalent might be the only test affected.

Better yet, only two questions, both in Data Analysis, central tendency.

WilCrofter commented 10 years ago

The two affected questions now work properly both in R 3.0.3 and R 3.1.0.

It does not appear, examining source code, that any other function or any questions other than the two mentioned earlier will be affected. I haven't thoroughly tested, but will make a pull request anyway.

ncarchedi commented 10 years ago

Addressed in @WilCrofter's pull request #125