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
aesthetic inheritance and annotation_custom #756
Comments
Weird. I'm not sure why |
Thanks for looking into this Hadley. The troubleshooting that led to this discovery really helped my understanding of how ggplot2 works under the hood, but this issue sure makes building custom functions with it tough! A quick look shows
I assume that |
Having reviewed Bryan's excellently written question on SO, including the MWE above,
Here
The error message I get it a bit different from what Bryan gets, but is probably the same underlying issue showing up differently. If I use
The basic issue seems to be that if one creates a I won't attempt my own MWE unless someone asks for it, because Bryan's example shows the problem well. |
It looks like starting with just an empty ggplot() call is critical when writing your own fairly complicated functions using ggplot2. In a rather cruel twist, I did not originally use an empty ggplot() call. I was having the problems described on SO in several places, then got what seemed to be really good advice from experienced folks to start with an empty call. Totally in the spirit of building up layers. Fixed some problems nicely, and conceptually is it much cleaner/clearer. But, I was still having the problems with annotation_custom etc. So I think I was having the same problem all along and any of my approaches might have worked in principle (but not in practice). fmitha has done a lot of valuable troubleshooting which I appreciate. Long story short, I think his conclusion is almost certainly spot-on. Hopefully someone can look into this in the near future. Bryan On May 13, 2013, at 2:51 PM, fmitha notifications@github.com wrote:
|
since no-one knows why |
On Mon, 13 May 2013, baptiste wrote:
I could certainly test it, if you think that is the only thing that is
|
The procedure is outlined here: https://github.com/hadley/ggplot2/wiki/Improving-the-ggplot2-documentation if you're familiar with installing ggplot2 with |
I patched 0.9.3.1 to change to Does anyone have any other ideas? I am not familar at all with the design of
|
This is related to #587, I think |
I had some comments here. My comments were sent by email, which messed up the formatting on the web page for some reason. I've reposted below, and deleted the version that was here. |
Outstanding and detailed analysis! When I had the problem, I had looked at the ggplot2 object and could see there was a difference between the pieces, but I didn't have the patience you applied to really ferret this one out. Great job, and a huge service to the community. Hopefully this can be evaluated soon and a fix issued. Bryan On May 21, 2013, at 5:39 PM, fmitha notifications@github.com wrote:
|
[Sending this by email resulted in a post with major formatting problems (not sure why), so I am reposting it, this time directly into the web page.] Here is an analysis of this issue (#756). Ggplot2 developers, I'd Bryan, if you don't want to read the whole thing, you can apply PATCH For the record, the original problem that brought me to this issue is Consider the following modified version of Bryan's code
PART 1: WHY PRINT GIVES AN ERROR FOR Here
It is not difficult to see what is going wrong. A fix is less obvious. The object
ggplot2 does not like it if the any of the data frames in the ################################################################## Say First
Then it calls
So,
This is a list of data frames. This is where the error is thrown,
contains two data frames, one of which is empty ( The code In this case, the data frame is
Since
################################################################## PART 2: ANNOTATION_CUSTOM LAYER HAS A NULL DATA ATTRIBUTE AND IS THAT A PROBLEM? We now look at what We can diagramatically represent the structure of
Here the data objects in When ggplot_build takes e.g. This happens at the beginning of Now, when the remaining functions in
where the second data frame corresponds to
This patch fixes
vs
i.e. the rest of the structure is the same. So, we return to the question, if it doesn't matter what data is used, Recall from our detailed discussion of PART 1, we observed at the end
was the reason that So, we have the curious situation that ggplot does not actually use Actually, if we take the It seems that the function ################################################################## The output of
The final result for
The output for
The final result for
As can clearly be seen from this, things appear to go wrong from In the case of
STEP 1: For each data attribute in
If so, it then replaces the STEP 2:If the data object constructed from above is either a waiver object or
otherwise it adds a
and returns. STEP 1 is the important step here. So to summarize what happens First, extract the layer data.
Second, pass In this case of In this case of ################################################################## |
@fmitha Thanks for the very detailed analysis. I don't get errors running your example code -- was that addressed by your fix for #587? So that seems to make PATCH 1 unnecessary. As for the second problem, the circle not rendering for
I don't know the exact source of the bug, but I think it should be possible to have a more targeted bug fix. When digging into this, I did find that there is a bug in
|
Hi Faheem - thanks for taking a look at this problem. I've replied on the On Tue, May 21, 2013 at 4:38 PM, Faheem Mitha faheem@faheem.info wrote:
|
Hi Winston, Thanks for the reply. On Thu, 23 May 2013, Winston Chang wrote:
By my example code, I assume you mean what I call EXAMPLE 1? If so, yes, the fix for #587 makes PATCH 1 unnecessary. They have the Of course, with just the #587 fix, the circle does not render for my Let me take this opportunity to ask - is it considered undesirable to
Oh, this clearly isn't the correct solution for this bug. I hoped it I'll take a look at your counterexample, and see if I can think of Did you see my comments about the fact that the rendering is
Ok. Are you going to commit this? BTW, I'd appreciate it if you could help me figure out why my tests
|
I spent some more time looking at the code. The following patch fixes Note: I noticed the unit tests failing far more often than the visual NB: If this patch or any later one is used, I want credit for it. Discussion follows.
DISCUSSION: I should first say I don't really understand the details of this code; (1) Create ggplot object by adding layers. This corresponds (2) Convert the ggplot object to an object than can be rendered. This (3) Build the pieces (grobs) of the plot, so it can be passed to Let us consider EXAMPLE 1. Getting rid of the bug described in Now, the function Consider the code inside
We see that the The second argument is ( As discussed earlier,
That means the result returned for the circle layer is empty. Even if Backing up a step, the
So, The situation is that the The bottom line is that it seems we need to copy some data to the ADDENDUM: I notice that Comments, corrections, and any other feedback welcome, of course. |
For the record, the patch given at the beginning of the previous posting breaks Winston Chang's example earlier in the thread. This example is:
|
This issue was closed without explanation, and I don't see anything in the repository that fixes it. Note there is a patch that (appears) to fix the problem, though I don't know if it is completely correct. If you want a pull request, please say so. |
Please resubmit a PR with minimal discussion. Unfortunately I don't have time to read this massive thread. |
Ok, will do. Should I do so on this issue, or create a new issue? |
New issue please. |
I posted this at SO and bring it here since it seems to be a real issue (and not just my confusion). SO post: http://stackoverflow.com/questions/14415073/inheritance-of-aesthetics-in-ggplot2-0-9-3-the-behavior-of-annotation-custom
I looked at the code for annotation_custom and it has a hard-wired inherit.aes = TRUE which I think is the problem. I don't see why this function needs any aesthetic at all. I did try several ways to override it and set inherit.aes = FALSE but I was unable to fully penetrate the namespace and make it stick.
Here is a MWE which illustrates the problem:
The text was updated successfully, but these errors were encountered: