ggobi / cranvas

Interactive graphics in R using Qt interfaces, a descendant of ggobi software
https://ggobi.github.io/cranvas.org/
80 stars 12 forks source link

qmap conflict #200

Open dicook opened 11 years ago

dicook commented 11 years ago

The example code for qmap:

library(ggmap) library(cranvas) qstate <- map_qdata("state") qmap(qstate, googleMap = TRUE)

creates a conflict, because there is a qmap in both packages.

chxy commented 11 years ago

That's why we load ggmap before cranvas - to mask the qmap from ggmap. If we load cranvas first, then have to use cranvas::map(qstate)

dicook commented 11 years ago

Yep, I realize that. It's really a nuisance to have the same function name in ggmap!

On Mar 12, 2013, at 12:18 PM, Xiaoyue Cheng wrote:

That's why we load ggmap before cranvas - to mask the qmap from ggmap. If we load cranvas first, then have to use cranvas::map(qstate)

— Reply to this email directly or view it on GitHub.


Di Cook visnut@gmail.com

tengfei commented 11 years ago

How about make cranvas depends and import on ggmap package in DESCRIPTION, and export your version of qmap() in NAMESPACE, that should fix the problem, only downside is you have to put ggmap in 'depends' as well as 'import' which looks not that formal.

On Tue, Mar 12, 2013 at 12:21 PM, dicook notifications@github.com wrote:

Yep, I realize that. It's really a nuisance to have the same function name in ggmap!

On Mar 12, 2013, at 12:18 PM, Xiaoyue Cheng wrote:

That's why we load ggmap before cranvas - to mask the qmap from ggmap. If we load cranvas first, then have to use cranvas::map(qstate)

— Reply to this email directly or view it on GitHub.


Di Cook visnut@gmail.com

— Reply to this email directly or view it on GitHubhttps://github.com/ggobi/cranvas/issues/200#issuecomment-14789402 .

Tengfei Yin MCDB PhD student 1620 Howe Hall, 2274, Iowa State University Ames, IA,50011-2274

yihui commented 11 years ago

How about using ggmap::qmap() instead of loading the package like library(ggmap)?

tengfei commented 11 years ago

If I understand the issue correctly, I think the potential problem is that, users may load ggmap package accidently after they load cranvas sometime(we cannot prevent this happen), that is going to override cranvas::qmap. To avoid this happens, we probably have to set ggmap in dependencies(import itself is not enough), so library(cranvas) with load ggmap automatically and mask ggmap::qmap(), even thought, users library(ggmap) after that, it is not going to override the qmap(), so they have to call ggmap::qmap to force use the function from ggmap when needed. Another add-on solution is that, if object for qmap() from both packages are different, we can define S3/S4 method for qmap, to dispatch method automatically, and call the right function.

On Tue, Mar 12, 2013 at 1:25 PM, Yihui Xie notifications@github.com wrote:

How about using ggmap::qmap() instead of loading the package like library(ggmap)?

— Reply to this email directly or view it on GitHubhttps://github.com/ggobi/cranvas/issues/200#issuecomment-14793804 .

Tengfei Yin MCDB PhD student 1620 Howe Hall, 2274, Iowa State University Ames, IA,50011-2274

yihui commented 11 years ago

That is probably an edge case. We need to guarantee our qmap() works in the examples. If the user has to load ggmap at the same time, it is his/her responsibility to solve the conflicts by ::.

dicook commented 11 years ago

True, except the code is supposed to help the user, particularly get up to speed with cranvas. If they accidently load the ggmap library second, then the code breaks, and there is no clue in the code that this is the problem. Either a comment should be added to the code sample, letting the user know that there is a reason why the order of the libraries is this way, or that the ggmap::qmap is explicitly given in the code rather than loading the library.

Do we even have ggmap as a dependency for cranvas??

On Mar 12, 2013, at 1:56 PM, Yihui Xie wrote:

That is probably an edge case. We need to guarantee our qmap() works in the examples. If the user has to load ggmap at the same time, it is his/her responsibility to solve the conflicts by ::.

— Reply to this email directly or view it on GitHub.


Di Cook visnut@gmail.com

tengfei commented 11 years ago

You are right, it depends, if that's just one case used in example, then your proposed method is totally fine and sufficient.

I have the same problem before, trying to override qplot()/autoplot() from ggplot2(), and turn it into a S4 method in ggbio. But the case for me is very different, users almost always load ggplot2 after they load ggbio, which caused the problem, so I have to use solution above.

On Tue, Mar 12, 2013 at 1:56 PM, Yihui Xie notifications@github.com wrote:

That is probably an edge case. We need to guarantee our qmap() works in the examples. If the user has to load ggmap at the same time, it is his/her responsibility to solve the conflicts by ::.

— Reply to this email directly or view it on GitHubhttps://github.com/ggobi/cranvas/issues/200#issuecomment-14795761 .

Tengfei Yin MCDB PhD student 1620 Howe Hall, 2274, Iowa State University Ames, IA,50011-2274

chxy commented 11 years ago

The only function I borrow from ggmap is get_googlemap, so I've changed it to ggmap::get_googlemap in the code.

ggmap is not a dependency but a suggestion to cranvas.

On Tue, Mar 12, 2013 at 2:03 PM, dicook notifications@github.com wrote:

True, except the code is supposed to help the user, particularly get up to speed with cranvas. If they accidently load the ggmap library second, then the code breaks, and there is no clue in the code that this is the problem. Either a comment should be added to the code sample, letting the user know that there is a reason why the order of the libraries is this way, or that the ggmap::qmap is explicitly given in the code rather than loading the library.

Do we even have ggmap as a dependency for cranvas??

On Mar 12, 2013, at 1:56 PM, Yihui Xie wrote:

That is probably an edge case. We need to guarantee our qmap() works in the examples. If the user has to load ggmap at the same time, it is his/her responsibility to solve the conflicts by ::.

— Reply to this email directly or view it on GitHub.


Di Cook visnut@gmail.com

— Reply to this email directly or view it on GitHubhttps://github.com/ggobi/cranvas/issues/200#issuecomment-14796169 .