Skip to content
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

Converting OverlapEncodings Rnw file to Rmd #24

Open
wants to merge 11 commits into
base: devel
Choose a base branch
from

Conversation

BerylKanali
Copy link

Hi @jwokaty
HTML and PDF for comparison.
RMD Build and RMD Check passed.

Tried to do some self review. Hopefully it will have less issues.to be changed.

Copy link
Contributor

@jwokaty jwokaty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work this vignette. It was quite long. There are a few issues to address within the document. Note that it's important to compare the HTML and the PDF to catch problems.

According to Marcel, the footnotes going to the bottom is related to a Pandoc issue, so we'll leave them as they are.

Please remove extra lines and format paragraphs with Code > Reflow comment.

person("Martin", "Morgan", role="aut"))
person("Martin", "Morgan", role="aut"),
person("Beryl", "Kanali", role="ctb",
comment="Converted 'OverlapEncodings' vignette from Sweave to RMarkdown."))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the same indentation as line 18.

vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
Comment on lines 1347 to 1369
Paired-end encodings `"2{-}{-}1:jm{-}{-}m:am{-}{-}m:af{-}{-}i:"`
(73 occurences in `U3.ovenc`),
`"1{-}{-}2:i{-}{-}jm:a{-}{-}am:a{-}{-}af:"` (53 occurences in
`U3.ovenc`), and
`"2{-}{-}2:jm{-}{-}mm:am{-}{-}mm:af{-}{-}jm:aa{-}{-}af:"`
(9 occurences in `U3.ovenc`), correspond to the following paired-end
overlaps:

- `"2{-}{-}1:jm{-}{-}m:am{-}{-}m:af{-}{-}i:"`

- paired-end read (1 skipped region on the first end, no skipped region
on the last end): ooo----------o oooo
- transcript: ... >>>>> >>>> >>>>>>>>> ...


- `"1{-}{-}2:i{-}{-}jm:a{-}{-}am:a{-}{-}af:"`

- paired-end read (no skipped region on the first end, 1 skipped region
on the last end): oooo oo---------oo
- transcript: ... >>>>>>>>>>> >>> >>>>>> ...


- `"2{-}{-}2:jm{-}{-}mm:am{-}{-}mm:af{-}{-}jm:aa{-}{-}af:"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare with original PDF. This section seems to have problems like the earlier section where braces surround hyphens.

vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
vignettes/OverlapEncodings.Rmd Outdated Show resolved Hide resolved
@hpages
Copy link
Contributor

hpages commented Jan 18, 2023

@BerylKanali @jwokaty Please make sure that things like this (from section 5.1.1):

overlap_encodings_5_1_1

or this (from section 5.2.1):

overlap_encodings_5_2_1

are rendered properly. I don't know how to do this, hopefully it's possible! 🤞

This is a tough vignette, but, AFAIK, not a very visited/popular one. I wonder if it's worth the effort of converting to Rmd 😞

Thanks,
H.

@BerylKanali
Copy link
Author

@BerylKanali @jwokaty Please make sure that things like this (from section 5.1.1):

overlap_encodings_5_1_1

or this (from section 5.2.1):

overlap_encodings_5_2_1

are rendered properly. I don't know how to do this, hopefully it's possible! 🤞

This is a tough vignette, but, AFAIK, not a very visited/popular one. I wonder if it's worth the effort of converting to Rmd 😞

Thanks, H.

@hpages We had noticed some elements in the PDF did not render properly when we compared it to the code, This is how section 5.2.1 appears now in the HTML file. The paired-end encodings appear in full, in the PDF they were somehow slashed. Let me know if this is okay.
Capture

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Jan 18, 2023

@BerylKanali
Hi Beryl,
The figures don't look quite like the ones above.
Perhaps you can use \preformatted{} tags?

@BerylKanali
Copy link
Author

@BerylKanali Hi Beryl, The figures don't look quite like the ones above. Perhaps you can use \preformatted{} tags?

Hi @LiNk-NY Do you mean these figures?
Capture4

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Jan 20, 2023

@BerylKanali Hi Beryl

Hi @LiNk-NY Do you mean these figures?
Capture4

Yes, the alignment should be maintained.

@jwokaty
Copy link
Contributor

jwokaty commented Jun 28, 2023

\preformatted{} doesn't seem to work, but <pre></pre> could work.

@hpages
Copy link
Contributor

hpages commented Mar 25, 2024

@jwokaty Is it ok to close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants