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

Fixed variable order in ggpairs #1

Merged
merged 7 commits into from Apr 14, 2011
Merged

Fixed variable order in ggpairs #1

merged 7 commits into from Apr 14, 2011

Conversation

eibanez
Copy link
Contributor

@eibanez eibanez commented Mar 12, 2011

Hello GGobi group!

My name is Eduardo and I'm one of Dr. Cook's students. Last semester she introduced me to GGally and, as I was using it, I found an error in the way the variables were displayed when using "ggpairs". I finally went through the code and I think I have fixed the issue. I modified the functions "ggpairs" and "plot_types". I believe that these changes don't raise other issues in the code.

You can use the following example to test the changes:

library(GGally)
a <- 1:4
b <- c(10,15,11,14)
c <- c(1000,999,996,990)
data <- cbind(a, b, c)
ggpairs(data)

The current version of "ggpairs" produces the following graphical output. We can check that the axes for each one of the scatterplots are not correct:
http://img863.imageshack.us/img863/4702/ggallyold.jpg

The changes I made in the code fixes the problem:
http://img827.imageshack.us/img827/9139/ggallynew.jpg

You can also verify the change by comparing the output to that of:
plot(data)

A byproduct of this correction is that the ticks in the horizontal axes for the first column are not aligned, but that is a separate problem.

Please let me know your comments.

Eduardo

@@ -261,7 +261,7 @@ ggpairs <- function(
ggpairsPlots <- list()


grid <- expand.grid(x = 1:ncol(data[columns]), y = 1:ncol(data[columns]))
grid <- expand.grid(y = 1:ncol(data[columns]), x = 1:ncol(data[columns]))[, 2:1]
Copy link
Member

Choose a reason for hiding this comment

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

It's a little clearer to use rev here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@hadley
Copy link
Member

hadley commented Mar 12, 2011

Looks great otherwise!

@schloerke
Copy link
Member

I took a look.

I ran the code you suggested on the latest version:
a <- 1:4
b <- c(10,15,11,14)
c <- c(1000,999,996,990)
data <- cbind(a, b, c)
ggpairs(data)

I had the same output that you want to show in the pull request.

If you took the class last fall (before Dec. 15th, 2010), this makes perfect sense. The error was present then, but it has been corrected since then. I switched the x and y variables around.

I guess I should use more obvious datasets than iris.

THANK YOU for making this aware to me though. I have never done a merge before, but you have set this up very nicely.

Best,
Barret

@eibanez
Copy link
Contributor Author

eibanez commented Mar 22, 2011

I just downloaded the last version (0.2.3) and the output is indeed the intended. I was making my previous changes by executing the code from the old version.

I see what you did in that commit. The reason why you had to switch x and y is that plot_types is not listing the plots correctly. ggpairs expects left-to-right and plot_types cycles top-to-bottom first.

I reverted your change and added it to the pull request. The output is still correct and now x and y are consistent when calling addAndOverwriteAes().

Feel free to either accept or ignore these change. The end result is the same.

@eibanez
Copy link
Contributor Author

eibanez commented Apr 12, 2011

All files are up to date. Please considering merging this.

Thanks!

@jcrowley11 jcrowley11 merged commit c429242 into ggobi:master Apr 14, 2011
@jcrowley11
Copy link
Contributor

Sorry it took me so long to get around to this Eduardo. I'm pretty new to GitHub and wasn't really sure what was going on with this "forking" and "pull request," and it took me too long to sit down and read through the guides. Thanks for the contribution!

Jason

@eibanez
Copy link
Contributor Author

eibanez commented Apr 14, 2011

No problem. Learning to use it myselft too. Thanks!

schloerke pushed a commit that referenced this pull request Feb 22, 2016
@sbihorel sbihorel mentioned this pull request Feb 25, 2018
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.

None yet

4 participants