Vignette typos fixes

[mattheaphy]

A few grammatical fixes, updates of old function / argument names, and example code updates in 3 vignettes.

Regarding code updates:

classes-objects.Rmd

In the example that included a getter and a setter for the Range class, the setter was getting called on object instantiation. This resulted in the supplied value of @end being overridden to an empty vector because @length's default value is empty. The resulting output of Range(start = 1, end = 10) was this:

<Range>
 @ start : num 1
 @ end   : num(0) # should be 10
 @ length: num(0) # should be 9

I updated the length setter to short circuit and return the original object when value is empty, and that gave me the correct output. I wanted to call this out because the maintainers might have a better solution in mind for handling instantiation matters like this.

compatibility.Rmd

I noticed that the more natural representation of the rle class using S7 was overwriting the rle() function, and this was creating incorrect output for rle(1:10).

Run Length Encoding
  lengths: int [1:10] 1 2 3 4 5 6 7 8 9 10 # should be all 1's
  values : logi(0) # should be 1-10

I used the name new_rle here instead that seemed to clean things up.

[hadley]

@t-kalinowski it probably makes sense for this to be an explicit is.null() test, right?

[t-kalinowski]

I don't think so. The default value passed in by the constructor will be double(0). If we want the default to be NULL, we'd have to change the property class to NULL | class_double, or implement a custom constructor.

[hadley]

Maybe rle2? I think calling it new_ is a bit confusing because the while the object is constructor, it's also the class.

[t-kalinowski]

I agree, rle2 would be clearer than new_rle.

[mattheaphy]

There's a few other instances north of this line where the name new_rle is used that would have to be updated as well for the examples to work as the original author intended. I'll add that change.

[t-kalinowski]

Thanks for catching this! After looking more closely, I see that new_rle() is indeed the best choice here. The intent is for rle() to serve as a convenient constructor, and we show how to redefine the underlying new_rle() from the “old-style” S3 approach to the S7 approach (in two different ways) while keeping the user-facing rle() constructor unchanged.

Strictly speaking, it might make sense to replace the user-facing rle() with an S7 rle class that has a custom constructor, but that would increase the example’s size and complexity without adding much clarity, in my opinion. I’ve restored the previous new_rle() approach.

[t-kalinowski]

Thank you for the edits!

[hadley]

Thank you!