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

theme_classic() no axis lines in version 2.1 #1565

Closed
jscamac opened this Issue Mar 2, 2016 · 24 comments

Comments

Projects
None yet
@jscamac

jscamac commented Mar 2, 2016

Shouldn't theme_classic actually have axis lines?
Is the update a mistake?

See theme_classic():
http://docs.ggplot2.org/dev/ggtheme.html

@Katiedaisey

This comment has been minimized.

Contributor

Katiedaisey commented Mar 2, 2016

It seems my update to allow access to lower level elements (axis.line.x, panel.grid.major.y, etc) leaves changes to upper level elements unused. Let me give this some thought.

@steveharoz

This comment has been minimized.

Contributor

steveharoz commented Mar 3, 2016

Here's a temporary workaround for the axes if anyone needs it until the issue's resolved:
theme_classic() + theme(
axis.line.x = element_line(colour = 'black', size=0.5, linetype='solid'),
axis.line.y = element_line(colour = 'black', size=0.5, linetype='solid'))

@jscamac

This comment has been minimized.

jscamac commented Mar 3, 2016

Thanks. I already had the work around but thought I should let you know of the changes to the classic_theme.

@ustervbo

This comment has been minimized.

ustervbo commented Mar 5, 2016

I think the problem is related to #1567 and seems to stem from complete removal of axis lines by default

@Katiedaisey

This comment has been minimized.

Contributor

Katiedaisey commented Mar 5, 2016

It has to do with how attributes for elements are inherited. To remove an attribute, like axis.line in some themes, the current code sets them as element_blank(). However if a subelement, axis.line.x and axis.line.y are added, they inherit element_blank and will not appear. Newly added elements need to inherit from the original object (line, rect, or text) skipping any blank elements but I'm still thinking on how to do that.

@bklingen

This comment has been minimized.

bklingen commented Mar 9, 2016

Really would appreciate for theme_classic() to work as in version 2.0.0 (and all prior versions), showing the x and y axis line as before instead of drawing no axis at all. (And thanks for the, hopefully temporary, workaround). I have countless shiny applications that depend on the classic theme, and re-programming all of these would be a nightmare.

@Katiedaisey

This comment has been minimized.

Contributor

Katiedaisey commented Mar 9, 2016

I think I have this completely fixed. Would anyone mind testing before I request a pull?

Katiedaisey added a commit to Katiedaisey/ggplot2 that referenced this issue Mar 9, 2016

@steveharoz

This comment has been minimized.

Contributor

steveharoz commented Mar 9, 2016

Hi @Katiedaisey. I pulled from your fork and tried running the example in http://docs.ggplot2.org/current/ggtheme.html
The ticks and labels look messed up, but I'm not sure if it's related.

ggplot(mtcars) + 
  geom_point(aes(x = wt, y = mpg, colour=factor(gear))) + 
  facet_wrap(~am) +
  theme_classic()

image

Nevertheless, axis lines appear to work:

ggplot(mtcars, aes(wt, mpg)) +
  geom_point() +
  theme(axis.line=element_line())

image

Katiedaisey added a commit to Katiedaisey/ggplot2 that referenced this issue Mar 10, 2016

Katiedaisey added a commit to Katiedaisey/ggplot2 that referenced this issue Mar 10, 2016

Katiedaisey added a commit to Katiedaisey/ggplot2 that referenced this issue Mar 10, 2016

@Katiedaisey

This comment has been minimized.

Contributor

Katiedaisey commented Mar 10, 2016

@steveharoz The ticks and labels were affected. They should also be fixed now.

@steveharoz

This comment has been minimized.

Contributor

steveharoz commented Mar 10, 2016

Nice! Looks like it's fixed.

Here are the graphs with latest commit:
image
image

Looks identical to commit a760ff6 (right before the theme updates)
image
image

@sjin09

This comment has been minimized.

sjin09 commented Mar 28, 2016

Dear Katiedaisy,

I have recently downloaded the ggplot2 again to my desktop and found that the theme_classic() does not draw the axis.line() and I have come across this post. I have tried as shown by steveharoz.

ggplot(mtcars, aes(wt, mpg)) +
geom_point() +
theme(axis.line=element_line())

Would this problem be addressed again?

Many Thanks,
Jin

@Katiedaisey

This comment has been minimized.

Contributor

Katiedaisey commented Mar 28, 2016

Hi @sjin09
You downloaded ggplot2 from CRAN or from my git? The fix has not yet been added to the package available on CRAN.

@ecp52

This comment has been minimized.

ecp52 commented May 19, 2016

Wizards of ggplot, I also have this problem, that the axes are not drawn even after downloading the package from this site.
Very grateful for your work on this!
image

@ecp52

This comment has been minimized.

ecp52 commented May 19, 2016

OK, had to get rid of the old version-- works great now!
Thanks for your hard work!
image

@bklingen

This comment has been minimized.

bklingen commented Jun 8, 2016

I still don't get axis lines:

ggplot(mtcars) + geom_point() + theme_classic()

On 3/9/2016 5:09 PM, Katiedaisey wrote:

I think I have this completely fixed. Would anyone mind testing before
I request a pull?

— Reply to this email directly or view it on GitHub
#1565 (comment).

@jac0bean9

This comment has been minimized.

jac0bean9 commented Jul 8, 2016

I'm having a similar trouble, for specifically requesting a grid. For example:

ggplot(mtcars, aes(wt, mpg)) + geom_point() + theme_classic() + theme(panel.grid.major = element_line(colour='red'))

Is there a solution? Should I open up a different issue for this?

@rpruim

This comment has been minimized.

rpruim commented Jul 8, 2016

This issue does not appear to be fixed in

 ggplot2    * 2.1.0       2016-07-08 Github (hadley/ggplot2@b181e9a) 

I get the following:

ggplot(aes(x = eruptions), data = faithful) + geom_histogram() + theme_classic()

image

@steveharoz

This comment has been minimized.

Contributor

steveharoz commented Jul 8, 2016

@Katiedaisey: Have you made a pull request with your fix?

Edit: Actually, I think it's this one - #1581
For folks wondering about the status of this bug, follow that pull request.

@Katiedaisey

This comment has been minimized.

Contributor

Katiedaisey commented Jul 12, 2016

@steveharoz: yes. It hasn't been integrated yet. No idea what's going on.

@jac0bean9 @rpruim: This has been fixed in my pull request #1581 and as available on my git. It just hasn't been integrated yet.

@assaforon

This comment has been minimized.

assaforon commented Jul 20, 2016

@Katiedaisey thank you so much for the fix. I am actually under a crunch to use it, as the PI in my study wants precisely the classical layout (black axes, no borders) for a rather elaborate 4x2 grid of ggplots I've written for an article we're submitting.
Being a novice Git user, I just copy-pasted the relevant functions you've modified in your edits, and am sourcing them directly after loading ggplot2. That didn't work. I changed the theme to 'theme_classic0', inheriting from 'theme_bw0'. Still doesn't work. I do see that the theme is defined as you had set it, but for some reason the output is still no axes rather than black ones.
How can I build ggplot2 from your edit rather than from the official one? Or any other advice as to how to make this work ASAP?

Thanks a bunch!
Assaf

@rpruim

This comment has been minimized.

rpruim commented Jul 20, 2016

It's a one-liner if you have devtools installed. A two-liner if not:

install.packages("devtools")
devtools::install_github("Katiedaisey/ggplot2")
@assaforon

This comment has been minimized.

assaforon commented Jul 20, 2016

Wow, it works now! Thank you both @rpruim @Katiedaisey.
I do use devtools, in fact I have 2 packages right here on GitHub, one of which ('cir') is only available via install_github() at the moment.
But I didn't realize Katie actually had a full working version of ggplot2 on her account. I am a very novice Git user, as I said ;)

Anyway, all's well that ends well. How can we up the pressure on Hadley et al. to finally integrate this?

@Katiedaisey

This comment has been minimized.

Contributor

Katiedaisey commented Jul 20, 2016

@assaforon Hope it works out for you. I'll look at updating the package on my account to the most recent version of ggplot2 incase there's any of the newer changes you need.

@hadley

This comment has been minimized.

Member

hadley commented Jul 29, 2016

Duplicate of #1567

@hadley hadley closed this Jul 29, 2016

thomasp85 added a commit that referenced this issue Sep 21, 2016

Add inherit.blank argument to element constructors (#1754)
Fixes #1555, #1557, #1565, and #1567

* Add inherit.blank argument to element constructors

* Look for inherit.blank when combining

* Set inherit.blank = TRUE automatically when theme is complete

@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.