Skip to content

fix: keep baseline scale to baseline 0 even if set to None #18663

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 2 commits into from
Oct 12, 2020

Conversation

andrzejnovak
Copy link
Contributor

@andrzejnovak andrzejnovak commented Oct 5, 2020

PR Summary

Ok hopefully last follow up on #18579. The current setup, unfortunately, ignores stickies when baseline=None
image

Updating datalim as well fixes this. Thanks @anntzer

image

@timhoffm @jklymak

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and pydocstyle<4 and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@anntzer
Copy link
Contributor

anntzer commented Oct 5, 2020

That's not really the intended semantics of sticky_edges. In particular, it's not even clear to me what behavior you expect if there are multiple sticky edges (do you expand to include all of them? that's not what the current implementation does, at least).
Can't you just include the baseline in the datalim? Something like self.dataLim.update_from_data(<some point on the baseline>). (The usual disclaimer about not having actually tried it.)

@andrzejnovak andrzejnovak changed the title feat: add option to force sticky_edges fix: keep baseline scale to baseline 0 even if set to None Oct 5, 2020
@andrzejnovak
Copy link
Contributor Author

Thanks @anntzer that works much better.

@dopplershift
Copy link
Contributor

Could you add some kind of test for this?

@andrzejnovak
Copy link
Contributor Author

@dopplershift fixed

@andrzejnovak
Copy link
Contributor Author

bump, the codecov report seems to complain about unrelated stuff

@timhoffm timhoffm added this to the v3.4.0 milestone Oct 12, 2020
@timhoffm timhoffm merged commit c3e96c5 into matplotlib:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants