Conversation
…links directing to the corresponding vignettes for datatable-intro.Rmd
…links directing to the corresponding vignettes + made small grammatical changes for datatable-secondary-indices-and-auto-indexing.Rmd
…links directing to the corresponding vignettes for datatable-joins.Rmd
…links directing to the corresponding vignettes for datatable-keys-fast-subset.Rmd
…links directing to the corresponding vignettes for datatable-reference-semantics.Rmd
…responding vignette for datatable-sd-usage.Rmd
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6617 +/- ##
=======================================
Coverage 98.60% 98.60%
=======================================
Files 79 79
Lines 14518 14518
=======================================
Hits 14316 14316
Misses 202 202 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
Two things that I can incorporate into this PR which I think are related or I hope not off-topic: (but need feedback/opinions) A) Removal of the mentions for the non-existent(?) 'design' vignette, e.g.: data.table/vignettes/datatable-intro.Rmd Line 317 in 03c647f B) Changing items of the form text: link to be hyperlinks, or of the format [text](link), e.g.: data.table/vignettes/datatable-joins.Rmd Lines 685 to 695 in 03c647f ## Reference
- - *Understanding data.table Rolling Joins*: https://www.r-bloggers.com/2016/06/understanding-data-table-rolling-joins/
+ - [*Understanding data.table Rolling Joins*](https://www.r-bloggers.com/2016/06/understanding-data-table-rolling-joins/)
- - *Semi-join with data.table*: https://stackoverflow.com/questions/18969420/perform-a-semi-join-with-data-table
+ - [*Semi-join with data.table*](https://stackoverflow.com/questions/18969420/perform-a-semi-join-with-data-table)
- - *Cross join with data.table*: https://stackoverflow.com/questions/10600060/how-to-do-cross-join-in-r
+ - [*Cross join with data.table*](https://stackoverflow.com/questions/10600060/how-to-do-cross-join-in-r)
- - *How does one do a full join using data.table?*: https://stackoverflow.com/questions/15170741/how-does-one-do-a-full-join-using-data-table
+ - [*How does one do a full join using data.table?*](https://stackoverflow.com/questions/15170741/how-does-one-do-a-full-join-using-data-table)
- - *Enhanced data.frame*: https://rdatatable.gitlab.io/data.table/reference/data.table.html
+ - [*Enhanced data.frame*](https://rdatatable.gitlab.io/data.table/reference/data.table.html) |
|
Also, do we need to ping |
| **Keys:** Actually `keyby` does a little more than *just ordering*. It also *sets a key* after ordering by setting an `attribute` called `sorted`. | ||
|
|
||
| We'll learn more about `keys` in the `vignette("datatable-keys-fast-subset", package="data.table")`; for now, all you have to know is that you can use `keyby` to automatically order the result by the columns specified in `by`. | ||
| We'll learn more about `keys` in the [*Keys and fast binary search based subset*](datatable-keys-fast-subset.html) vignette; for now, all you have to know is that you can use `keyby` to automatically order the result by the columns specified in `by`. |
There was a problem hiding this comment.
to address off-line use, #6612 (comment)
can this be changed from
[*Keys and fast binary search based subset*](datatable-keys-fast-subset.html)
to
[`vignette("datatable-keys-fast-subset", package="data.table")`](datatable-keys-fast-subset.html)
which renders as vignette("datatable-keys-fast-subset", package="data.table")
There was a problem hiding this comment.
Certainly, but what do you suggest for rephrasing the text around it? or should it stay the same? (it reads a bit odd, but it might just be me)
For e.g., in the case you highlighted above, it currently reads as '... in the Keys and fast binary search based subset vignette ...' so should I change it to:
i) ' ... in vignette("datatable-keys-fast-subset", package="data.table") ...' (discarding the word 'vignette' for repetition)
ii) or ' ... in the vignette("datatable-keys-fast-subset", package="data.table") ...' (sounding more natural as in 'the vignette')
iii) or simply ' ... in the vignette("datatable-keys-fast-subset", package="data.table") vignette ...'
There was a problem hiding this comment.
whatever is easiest is fine with me
if you have time to make it sound natural, that is great
There was a problem hiding this comment.
Thanks! I think the current version looks fine and does a good job in considering both points (hyperlinks, off-line use) so I'm cool with it (let me know if you need any other changes)
|
great thanx |
|
here is some code I used to check that the text and link are consistent, vignette.line.vec <- c(
'We have seen this example already in the `vignette("datatable-reference-semantics", package="data.table")` and the `vignette("datatable-keys-fast-subset", package="data.table")`.',
'We have seen this example already in the vignettes [`vignette("datatable-reference-semantics", package="data.table")`](datatable-reference-semantics.html) and [`vignette("datatable-keys-fast-subset", package="data.table")`](datatable-keys-fast-subset.html).',
'We will discuss fast *subsets* using keys and secondary indices to *joins* in the next vignette, `vignette("datatable-joins", package="data.table")`.',
'We will discuss fast *subsets* using keys and secondary indices to *joins* in the [next vignette (`vignette("datatable-joins", package="data.table")`)](datatable-joins.html).')
Rmd.line.list <- list()
for(f in Sys.glob("*.Rmd")){
Rmd.line.list[[f]] <- readLines(f)
}
Rmd.line.vec <- do.call(c, Rmd.line.list)
vignette.line.vec <- grep("vignette(",Rmd.line.vec,fixed=TRUE,value=TRUE)
nc::capture_all_str(
vignette.line.vec,
'\\[',
text=".*?",
'\\]\\(',
vig_name='datatable-.*?',
'.html'
)[
, pattern := sprintf('`vignette("%s", package="data.table")`', vig_name)
][
, .(expected=grepl(
pattern,
text, fixed=TRUE)
),
by=.(text,pattern)
]if links are consistent then we get TRUE in expected column. |
|
Thanks for sharing, neat stuff with |
|
This is nice! I may have missed something -- is there a plan to prevent regression here (i.e., any future vignettes/updates to vignettes which generate new references to other vignettes, which do not wind up linked in this way)? |
|
good idea, is that something we can enforce via lint? |
|
Yea, we can probably do something for most cases with a simple regex (probably in the The case changed here for |
Closes #6612