This repository has been archived by the owner on Jun 21, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update oncoprints to use histology-specific goi lists #1046
Update oncoprints to use histology-specific goi lists #1046
Changes from 7 commits
5d0bcf0
a5e5af1
da63721
7e28741
bdc3319
9cbd240
d4c2e51
c9a1f8a
c51132c
b39e417
e64290f
567effe
f735ceb
04874a4
52ac162
ede7838
ea6e207
44cf639
f889ad5
3ad02de
d71f76b
34c3b16
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The actual code changes I'm suggesting are untested, but I believe you will only have to take these steps if someone specifies a GOI list and a top n, so you can simplify this a bit:
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.
Upon testing this method does not appear to obey the
top_n
argument.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.
Can you expand on that a bit?
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.
It does not limit the amount of genes being displayed to
top_n
, it instead shows all of the genes listed in the goi list (the behavior it previously exhibited but we did not want).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.
But the two
if
way worked?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.
That's correct.
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.
That suggests that the logic isn't quite right because it's using the original list (
goi_list
) - the conditions for theif()
are not being met. Since you don't want to subset the MAF and do themafSummary()
steps unless you have to and you only have to if you have atop_n
argument I would see if you can get to the bottom of it.