Skip to content

axisbelow should just set zorder. #9094

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.

Sign up for GitHub

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

Jump to bottom
Merged
tacaswell merged 2 commits into from
Jul 1, 2018
Merged

axisbelow should just set zorder. #9094

tacaswell merged 2 commits into from
Jul 1, 2018

Conversation

@anntzer
Copy link
Contributor

@anntzer anntzer commented Aug 26, 2017 •
edited
Loading

See discussion starting at https://gitter.im/matplotlib/matplotlib?at=59a0b3b9bc46472974c206bf.

Please vote as to whether we should just deprecate the old axisbelow functions in favor of just using zorder everywhere (I think we should).

xref #5045 #6287

attn @afvincent

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@afvincent
Copy link
Contributor

afvincent commented Aug 26, 2017

FWIW, the float option for axisbelow seems to already have been evaluated in a not too old past here #6287

@anntzer
Copy link
Contributor Author

anntzer commented Aug 26, 2017

Actually I decided to not support float values in axisbelow -- just set zorder if that's what you want to do (in fact, one should just set zorder in all cases).

@tacaswell tacaswell added this to the 2.2 (next next feature release) milestone Aug 26, 2017
@anntzer anntzer force-pushed the axisbelow-zorder branch 3 times, most recently from 3a44d35 to ce41543 Compare August 26, 2017 07:45
@anntzer
Copy link
Contributor Author

anntzer commented Aug 26, 2017 •
edited
Loading

It appears that commenting out the lines

            if not np.isfinite(posx) or not np.isfinite(posy):
                raise ValueError("posx and posy should be finite values")

in text.py makes the currently failing docs example (custom_scale_example) just build fine... strange.

Edit: should be fixed now, was a silly error on my side.

@anntzer anntzer force-pushed the axisbelow-zorder branch 2 times, most recently from 36b2071 to 83771cc Compare August 26, 2017 20:19
@anntzer anntzer mentioned this pull request Nov 14, 2017
Closed
@anntzer anntzer modified the milestones: needs sorting, v3.0 Feb 15, 2018
@anntzer anntzer force-pushed the axisbelow-zorder branch from 19b0731 to 3184287 Compare June 30, 2018 13:11
NelleV
NelleV reviewed Jun 30, 2018
View reviewed changes
lib/matplotlib/axes/_base.py Outdated
----------
b : bool
..
ACCEPTS: bool
Copy link
Member

@NelleV NelleV Jun 30, 2018

Choose a reason for hiding this comment

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

Why did you add this line?

Copy link
Contributor Author

@anntzer anntzer Jun 30, 2018

Choose a reason for hiding this comment

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

... because the PR dates from before we got rid of them :)
Removing it.

@anntzer anntzer force-pushed the axisbelow-zorder branch from 3184287 to 0f4b615 Compare June 30, 2018 20:41
@NelleV
Copy link
Member

NelleV commented Jun 30, 2018

Apart from minor documentation problems, this looks good on the mpl side of things. I'm trying to stay away from the 3d stuff, so I can't comment on that.
Can I push directly to your branch to fix the couple of doc problems?

@NelleV NelleV requested a review from WeatherGod June 30, 2018 20:54
@anntzer anntzer force-pushed the axisbelow-zorder branch from 0f4b615 to 51f8468 Compare June 30, 2018 21:54
NelleV
NelleV approved these changes Jun 30, 2018
View reviewed changes
Copy link
Member

@NelleV NelleV left a comment

Choose a reason for hiding this comment

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

LGTM

@tacaswell tacaswell merged commit 4b316b9 into matplotlib:master Jul 1, 2018
@anntzer anntzer deleted the axisbelow-zorder branch July 1, 2018 05:04
@NelleV
Copy link
Member

NelleV commented Jul 1, 2018

🍾

@ImportanceOfBeingErnest ImportanceOfBeingErnest mentioned this pull request Oct 24, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NelleV NelleV NelleV approved these changes

@WeatherGod WeatherGod Awaiting requested review from WeatherGod

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@anntzer @afvincent @NelleV @tacaswell