-
Notifications
You must be signed in to change notification settings - Fork 10
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
Convert IRangesOverview.Rnw to IRangesOverview.Rmd #49
base: devel
Are you sure you want to change the base?
Conversation
Make sure that current indents matches the existing document
We will be using this checklist to review my PR I have reviewed it myself, Please review my changes and let me know if any more changes are required any suggestion will be appreciated. Note: During the review I noticed that pdf has
I Remember that One of the requirements of a vignette is that it must not install anything without the user's knowledge. But the code label uses
|
vignettes/IRangesOverview.Rmd
Outdated
|
||
```{r ranges-union} | ||
``` | ||
|
||
```{r ranges-punion} | ||
``` | ||
|
||
```{r ranges-intersect} | ||
``` | ||
|
||
```{r ranges-pintersect} | ||
``` | ||
|
||
```{r ranges-setdiff} | ||
``` | ||
|
||
```{r ranges-psetdiff} | ||
``` | ||
|
||
<!-- \subsection{Mapping Ranges Between Vectors} --> |
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.
@hpages This is just a suggestion, but maybe this section 366-385 can be removed?
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 have not yet removed it. I will remove it after receiving confirmation from @hpages.
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.
Sure, please remove. These are placeholders that were intended to be filled with something at some point but it never happened. Thx!
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.
Done
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.
Hi @sonali8434 Thank you for your PR. I noticed a few more places where pandoc didn't convert an expression correctly. Otherwise, everything looks really good and it will be ready for Herve after you address these changes.
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.
Thanks, @sonali8434! I think this ready for @hpages.
@jwokaty please review this PR I have reviewed it once using the checklist and make sure that
R CMD build
contains html. After your review we will ask @hpages for final review.Note: The plots position is quite different in the html document but I believe that in the pdf plots are appeared at the middle of the code blocks which is not possible to generate in html the current position of plots look good to me.
Attached
PDF
HTML for your reference.