Skip to content

ENH: add axisbelow option 'line', make it the default #6287

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

Merged
merged 5 commits into from
May 2, 2016

Conversation

efiring
Copy link
Member

@efiring efiring commented Apr 11, 2016

The present axisbelow value can be only True, which puts the ticks and gridlines below patches, or False, which puts them above lines. It might make more sense to put them above patches (e.g. filled contours) and below lines. This PR adds that option, and makes it the default. See #5980 and #4243.

Ideally we would be able to control ticks and gridlines separately, as requested in #4243, but this would require major refactoring. Ticklines and gridlines are drawn by Tick.draw which is called by Axis.draw. Hence the order in which they are drawn relative to other plot elements is controlled by Axes.xaxis.zorder etc., not by the zorder of each gridline, for example.

@efiring efiring added this to the 2.0 (style change major release) milestone Apr 11, 2016
@@ -981,7 +992,7 @@ def validate_animation_writer_path(p):
'errorbar.capsize': [0, validate_float],

# axes props
'axes.axisbelow': [False, validate_bool],
'axes.axisbelow': ['line', validate_axisbelow],
Copy link
Member

Choose a reason for hiding this comment

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

also need to change in the template

@tacaswell
Copy link
Member

I am moderately 👍 on this. Would it be better to let the rcparam be a float ?

@efiring
Copy link
Member Author

efiring commented Apr 11, 2016

I thought about the float option, but using strings fits better with the (perhaps unfortunate) existing name of the parameter/kwarg. "axisbelow line" makes a little bit of sense; "axisbelow 1.5" doesn't.
I originally took a stab at setting zorder of the gridlines, but it didn't work, for the reason given in the commit comment above.

@efiring
Copy link
Member Author

efiring commented Apr 11, 2016

There are 3 image failures involving minuscule differences. In the one I reproduced, I can't even see anything other than solid black in the diff image. I would not expect this PR to affect any images because the classic style is unchanged.

@WeatherGod
Copy link
Member

I have been seeing tiny image comparison failures locally. The diff images
show me all black, too.

On Sun, Apr 10, 2016 at 11:07 PM, Eric Firing notifications@github.com
wrote:

There are 3 image failures involving minuscule differences. In the one I
reproduced, I can't even see anything other than solid black in the diff
image. I would not expect this PR to affect any images because the classic
style is unchanged.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#6287 (comment)

@tacaswell
Copy link
Member

did some of the pgf tests avoid getting run in 'classic' mode?

@jenshnielsen
Copy link
Member

Yes the pgf tests have their own slightly different image comparison for some reason. They will not run in classic mode

@efiring
Copy link
Member Author

efiring commented Apr 11, 2016

The one Travis failure is in the docs build. I don't understand it.

@efiring
Copy link
Member Author

efiring commented Apr 15, 2016

The Travis failure is in the docs build, ./examples/api/custom_scale_example.py. That's a pretty complicated example, but I don't see how its failure could be related to the present PR. @mdboom, any ideas?

@efiring
Copy link
Member Author

efiring commented Apr 15, 2016

The example fails with rcParams['axes.axisbelow'] = True as well as with the new default, so it has always been broken--this PR is just triggering the failure.
The line in the example that triggers the failure (which occurs at draw time) is plt.ylabel('Latitude').

@efiring
Copy link
Member Author

efiring commented Apr 18, 2016

I think this is complete now. The last thing I added is a note in whats_new/style_changes. The change is visually so subtle that maybe it doesn't need to be mentioned here; if that's the case, the last commit can be removed.

@mdboom
Copy link
Member

mdboom commented Apr 25, 2016

👍 after adding a test.

@tacaswell tacaswell merged commit 515ba4b into matplotlib:master May 2, 2016
tacaswell added a commit that referenced this pull request May 2, 2016
ENH: add axisbelow option 'line', make it the default
@tacaswell
Copy link
Member

backported to v2.x as 2ec3780

@afvincent afvincent mentioned this pull request Aug 26, 2017
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants