matloff / fasteR

Fast Lane to Learning R!
958 stars 154 forks source link

abline is a not an S3 generic and can be used better. #6

Closed ReeceGoding closed 3 years ago

ReeceGoding commented 3 years ago

I was recently reading the S3 section and found a number of surprises that I would like to bring to your attention:

  1. The section's first example is abline, suggesting that it is an S3 generic. This is not the case. If you type abline in to your console, you will find that its generic behaviour is hard-coded.
  2. The section eventually calls cfs <- lmout$coefficients; abline(a = cfs[1], b = cfs[2]). This is a non-example of abline's generic behavior, it is only showing the reader how to extract the coefficients that the section used in its first example. An actual demonstration of its generic behaviour would be abline(lmout).
  3. Although abline is a non-example, str is an S3 generic used in this section. No attention was drawn to this fact.
  4. The claim that "An S3 object is actually an R list" is false. Factor variables are the most obvious counterexample.
  5. The question at the end asks about the effect of adding 100 pounds of weight to a model built from the mtcars data set. I do not believe that there is anything in that data set showing the user what unit the weights are in. 100 seems like a huge number to consider for a column that has a maximum value of 5.424. The units used might be hidden somewhere in the documentation, but a beginner's tutorial shouldn't require the user finding those facts.

Otherwise, good job! I've been enjoying this read.

matloff commented 3 years ago

Thanks for the valuable comments! Most are spot on, and I will correct the problems. I do differ with you on two points. First, AFAIK no one considers an R factor to be an S3 object. Second, regarding the 'mtcars' dataset, the name alludes to the fact that the data comes from Motor Trend, a popular American car magazine in the 1960s/70s; the weights are almost certainly in pounds, just like the gas mileage is in miles/gallon rather than say km/liter.

ReeceGoding commented 3 years ago

Addressing your points: 1) In my experience, people do consider factors to be S3 objects. This is exactly why a lot of R tutorials warn about how they sometimes act like integers and sometimes act like characters. A well-known example is section 8.2. of the R Inferno. The documentation for factors also contains many such warnings. Regardless, whether or not factors are S3 objects is not a matter of opinion. Run methods(class=factor) and you will see many generic functions that dispatch to them. 2) According to ??datasets::mtcars, the wt variable is "Weight (1000 lbs)". I think we're both right here. Yes, they're pounds, but the reader ought to be told that adding 100 pounds means adding 0.1.

matloff commented 3 years ago

R factors can be both very powerful and highly annoying. In my 'regtools' package (here on GitHub), I actually have 5 or 6 utilities to deal with problems arising with factors. But I've never heard anyone consider factors to be S3. I've been active in R since almost the beginning (and was an S user before R), have served in editorial capacity with 2 R-related journals etc.