Skip to content

ADd option to include identity line in pairs plots; select points to …#34

Merged
csoneson merged 3 commits intomasterfrom
update-plotpairs
Sep 25, 2022
Merged

ADd option to include identity line in pairs plots; select points to …#34
csoneson merged 3 commits intomasterfrom
update-plotpairs

Conversation

@csoneson
Copy link
Copy Markdown
Collaborator

…label in result plots based on nominal p-value to avoid ties.

…label in result plots based on nominal p-value to avoid ties.
@csoneson csoneson requested a review from mbstadler August 30, 2022 18:35
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 18, 2022

Codecov Report

Merging #34 (84ba630) into master (88bce82) will decrease coverage by 0.20%.
The diff coverage is 91.07%.

@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
- Coverage   97.10%   96.90%   -0.21%     
==========================================
  Files          20       20              
  Lines        2176     2196      +20     
==========================================
+ Hits         2113     2128      +15     
- Misses         63       68       +5     
Impacted Files Coverage Δ
R/plotPairs.R 82.52% <73.68%> (-2.87%) ⬇️
R/plotResults.R 98.34% <100.00%> (+0.08%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Collaborator

@mbstadler mbstadler left a comment

Choose a reason for hiding this comment

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

All great here. I have added a small rewording in the argument documentation of the plotVolcano and plotMeanDiff functions, feel free to revert if you prefer to original wording.
I also got all green tests under arm64. The coverage of plotPairs.R (based on covr::report()) is incomplete, but the output makes no sense (e.g. it is calling smoothscat 15 times, but the sole expression in the function body is reported as uncovered), so I think there is not much we can do and this is probably an issue in covr, rather than a true positive.

@csoneson
Copy link
Copy Markdown
Collaborator Author

Yes, I'm also not sure what's happening with the coverage in plotPairs (I think it was always off, so not related to this change). Agree with the rewording, will merge once the checks have run.

@csoneson csoneson merged commit c1fc918 into master Sep 25, 2022
@csoneson csoneson deleted the update-plotpairs branch September 25, 2022 10:59
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.

3 participants