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

Merged
merged 5 commits into from May 2, 2016

Conversation

Projects
None yet
6 participants
Owner

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.

@tacaswell tacaswell commented on the diff Apr 11, 2016

lib/matplotlib/rcsetup.py
@@ -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],
@tacaswell

tacaswell Apr 11, 2016

Owner

also need to change in the template

Owner

tacaswell commented Apr 11, 2016

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

Owner

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.

Owner

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.

Member

WeatherGod commented Apr 11, 2016

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)

Owner

tacaswell commented Apr 11, 2016

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

Owner

jenshnielsen commented Apr 11, 2016

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

Owner

efiring commented Apr 11, 2016

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

efiring added some commits Apr 11, 2016

@efiring efiring ENH: add axisbelow option 'line', make it the default
4485888
@efiring efiring replace 3 test images with tiny changes
6be8fb2
Owner

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?

Owner

efiring commented Apr 15, 2016 edited

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 efiring work around bug in custom_scale_example by setting axisbelow to False
069c4dc
@efiring efiring add a note to whats_new/style_changes.rst
9366510
Owner

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.

Owner

mdboom commented Apr 25, 2016

👍 after adding a test.

@efiring efiring add image test for the 3 axisbelow options
efffe26

@tacaswell tacaswell merged commit 515ba4b into matplotlib:master May 2, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.007%) to 69.665%
Details

tacaswell removed the needs_review label May 2, 2016

@tacaswell tacaswell added a commit that referenced this pull request May 2, 2016

@tacaswell tacaswell Merge pull request #6287 from efiring/axisbelow-lines
ENH: add axisbelow option 'line', make it the default
2ec3780
Owner

tacaswell commented May 2, 2016

backported to v2.x as 2ec3780

efiring deleted the efiring:axisbelow-lines branch Jun 23, 2016

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