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

adding some user friendly declarations! #15

Merged
merged 2 commits into from
Jan 29, 2020
Merged

adding some user friendly declarations! #15

merged 2 commits into from
Jan 29, 2020

Conversation

jaybee84
Copy link
Collaborator

No description provided.

@allaway
Copy link
Collaborator

allaway commented Jan 27, 2020

Just checking, are you still working on this or waiting on review?

@jaybee84
Copy link
Collaborator Author

Waiting on review on this one... working on a different branch for the next part of this module

Copy link
Collaborator

@allaway allaway left a comment

Choose a reason for hiding this comment

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

It seems like the lollipop plot no longer works, at least on my end...

R/mod_gene_var.R Show resolved Hide resolved
box(title = "Positional information of variants in tumor samples",
status = "primary", solidHeader = TRUE,
width = 1000,
collapsible = FALSE,
plotOutput(ns('lollipop_plot'))
)
plotOutput(ns('lollipop_plot')))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I get this error: "argument "p" is missing, with no default" with the full cohort selected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmm... I am not getting that error message on my end.

In previous iterations, I got this error when the cohort excluded all samples with genomic var data and the plotting dataset consisted of 0 samples. But in this case the new user messages should take care of this scenario.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I found the root of this bug: PoisonAlien/maftools#387

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

awesome!

also, I was able to reproduce the bug on my end when I pulled the develop branch today. On the gene_var_1 branch its works well but not on the develop :) possibly due to the issue you mentioned above :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Re-opening this conversation since this bug is now bugging me! :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To clarify my not-so-clear remark earlier: Acc to issue 387, the issue of maftools layout being overwritten by plotly was fixed in the latest version. But even with the latest version pulled from github, I am encountering this bug when I run the app locally. I reported this observation to PoisonAlien.

@allaway
Copy link
Collaborator

allaway commented Jan 29, 2020

I can't reproduce this bug on my computer at work....so I guess I was doing something weird, or pulled the wrong branch again. so, LGTM!

@allaway allaway merged commit 080fbf3 into develop Jan 29, 2020
@allaway allaway mentioned this pull request Jan 30, 2020
@allaway allaway deleted the gene_var_1 branch June 18, 2020 19:51
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.

2 participants