omarwagih / ggseqlogo

Publication-quality sequence logos in R
208 stars 30 forks source link

Facet plots in `geom_logo` should follow ggplot2 conventions #14

Closed MattBrauer closed 5 years ago

MattBrauer commented 5 years ago

Plotting multiple sequence logos is accomplished by passing a named list of sequences. ASAIK, this is not a standard mechanism for multiple plots.

What I'd like to do is something that aligns more with the tidy workflow, like: seq_tbl %>% ggplot() + geom_logo(sequence) + theme_logo() + facet_wrap(~sample, ncol=2)

rather than: ggplot() + geom_logo(list_of_sequences) + theme_logo() + facet_wrap(~seq_group, ncol=2)

where sequence and sample are both columns in my tibble.

As it now stands, I need to pass a named list of sequences and use a mystery variable (seq_group?) for the faceting.

Also, is there any way to set the order of the plots in the facet_wrap or facet_grid?

Thanks.

omarwagih commented 5 years ago

I agree it would be more flexible and in line with gg convention. The initial implementation of the package was never meant to allow for interfacing with geom_logo/ggplot2. It was only a wrapper, which is now the functionggseqlogo. I added the support for that later on to allow more flexibility customizing the plot.

I don't have much time to make changes, but you are welcome to submit a pull request if you'd like to take a stab at it.

MattBrauer commented 5 years ago

Hi Omar,

Thanks for getting back to me. I totally get it, and hope I did not come across as critical—I’m very appreciative of your package.

Someday maybe I’ll dig into the ggplot OO internals and take a stab at it!

Regards, Matt

On Aug 19, 2019, at 9:58 PM, Omar Wagih notifications@github.com wrote:

I agree it would be more flexible and in line with gg convention. The initial implementation of the package was never meant to allow for interfacing with geom_logo/ggplot2. It was only a wrapper, which is now the functionggseqlogo. I added the support for that later on to allow more flexibility customizing the plot.

I don't have much time to make changes, but you are welcome to submit a pull request if you'd like to take a stab at it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/omarwagih/ggseqlogo/issues/14?email_source=notifications&email_token=AAMFHTUWL6ZRXHPVFXG7VMDQFN2XDA5CNFSM4INLRFDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4VBXOA#issuecomment-522853304, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMFHTTXQDUG7Q7NUC43LHLQFN2XDANCNFSM4INLRFDA.

omarwagih commented 5 years ago

Not at all- I always welcome feedback! :)

Forgot to response to your question: the order of the facet is the same order of the list so you would just change that. e.g.

require(ggseqlogo)
data(ggseqlogo_sample)
ggseqlogo( seqs_dna[c(1, 2, 3)], ncol = 1 )
ggseqlogo( seqs_dna[c(3, 1, 2)], ncol = 1 )

Hope that helps!

MattBrauer commented 5 years ago

Much obliged, thanks.

On Aug 19, 2019, at 10:30 PM, Omar Wagih notifications@github.com wrote:

Not at all- I always welcome feedback! :)

Forgot to response to your question: the order of the facet is the same order of the list so you would just change that. e.g.

require(ggseqlogo) data(ggseqlogo_sample) ggseqlogo( seqs_dna[c(1, 2, 3)], ncol = 1 ) ggseqlogo( seqs_dna[c(3, 1, 2)], ncol = 1 ) Hope that helps!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/omarwagih/ggseqlogo/issues/14?email_source=notifications&email_token=AAMFHTVDKJJSDPR5NKEA5CDQFN6P3A5CNFSM4INLRFDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4VDIFQ#issuecomment-522859542, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMFHTQMUQ3KGXHX63IFGCDQFN6P3ANCNFSM4INLRFDA.