-
Notifications
You must be signed in to change notification settings - Fork 37
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
Vignette typos fixes #492
Vignette typos fixes #492
Changes from 4 commits
02b99d4
699917f
c880d80
8a22685
c68a17a
870524b
d3ecc77
fc312c4
5fb442f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -109,7 +109,7 @@ rle(1:10) | |
Alternatively you could convert it to the most natural representation using S7: | ||
|
||
```{r} | ||
rle <- new_class("rle", properties = list( | ||
new_rle <- new_class("rle", properties = list( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a few other instances north of this line where the name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for catching this! After looking more closely, I see that Strictly speaking, it might make sense to replace the user-facing |
||
lengths = class_integer, | ||
values = class_atomic | ||
)) | ||
|
@@ -118,7 +118,7 @@ rle <- new_class("rle", properties = list( | |
To allow existing methods to work you'll need to override `$` to access properties instead of list elements: | ||
|
||
```{r} | ||
method(`$`, rle) <- prop | ||
method(`$`, new_rle) <- prop | ||
rle(1:10) | ||
``` | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@t-kalinowski it probably makes sense for this to be an explicit
is.null()
test, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. The default value passed in by the constructor will be
double(0)
. If we want the default to beNULL
, we'd have to change the property class toNULL | class_double
, or implement a custom constructor.