Prevents axes limits from being resized by axes.fill_between by olinjohnson · Pull Request #25712 · matplotlib/matplotlib

olinjohnson · 2023-04-18T01:04:17Z

PR Summary

Closes #25682 by preventing Axes.fill_between() from automatically adjusting the axes limits when using axes coordinates. When axes coordinates are not in use and thus no transformation is passed to Axes.fill_between(), then adjusting the axes limits is necessary, but previously, in some cases, a transformation might have been in use and using axes coordinates could have caused the axes limits to be resized improperly.

PR Checklist

Documentation and Tests

Has pytest style unit tests (and pytest passes)
[N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).
[N/A] New plotting related features are documented with examples.

Release Notes

[N/A] New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
[N/A] API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
[N/A] Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

github-actions

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

ksunden · 2023-04-18T04:06:23Z

This could probably use a test of this behavior, something like the code from #25682

I don't think it needs to be an image comparison test, just an assertion that the limits are the same before/after the fillbetween call

QuLogic · 2023-04-18T06:04:39Z

+        if "transform" not in kwargs:
+            self.update_datalim(pts, updatex=True, updatey=True)


This should probably check the transform branches, similar to how lines do:

matplotlib/lib/matplotlib/axes/_base.py

Line 2354 in 690884f

updatex, updatey = line_trf.contains_branch_seperately(self.transData)

That may be a bit more complete, yeah... for reference I based the original suggestion on axline:

matplotlib/lib/matplotlib/axes/_axes.py

Lines 910 to 913 in b86ebba

if "transform" in kwargs:

# if a transform is passed (i.e. line points not in data space),

# data limits should not be adjusted.

datalim = []

@QuLogic I'm not yet very well-versed with transforms - wondering if you could provide some guidance as to how this would work?

I will continue to look into this as well.

See #25324 for a similar application of the transform branches for update_datalim on vlines/hlines. (Though I don't know for sure that you also need to handle rectilinear specifically as is done in that PR; just the transform bit might be enough.)

@QuLogic @ksunden I updated the PR to check transform branches - can you review this to make sure everything looks alright?

olinjohnson · 2023-04-20T23:50:40Z

@ksunden I did a few basic image tests after initially making the changes:

As you can see, the example using the code from the documentation remains the same (as expected), and the example using the code for reproduction from #25712 returns the expected output after the change.

Additionally, I also tested with an assertion (per your recommendation) which verified that the axes limits remain the same before and after the fill_between call.

original_lims = (ax.get_xlim(), ax.get_ylim())

ax.fill_between(x, 0, 1, where=y > threshold2,
                color='green', alpha=0.5, transform=ax.get_xaxis_transform())

assert (ax.get_xlim(), ax.get_ylim()) == original_lims

ksunden · 2023-04-21T01:57:33Z

@olinjohnson could you make this a unit test in lib/matplotlib/tests/test_axes.py?

https://matplotlib.org/stable/devel/testing.html#writing-a-simple-test

This will help us to ensure that the expected behavior is retained through future changes.

olinjohnson · 2023-04-21T18:03:41Z

Yes! I added the unit test - sorry, should have done this before.

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

ksunden · 2023-04-28T02:52:05Z

Thank you for your contribution, hope to hear from you again!

Pin 690884facc1e80b40640649f2a64ceab67afb42e and its regression selectors for reproducible runs. Source: matplotlib/matplotlib#25712

fixed issue matplotlib#25682

cb9b2ef

olinjohnson changed the title ~~fixed issue #25682~~ Fixes #25682 - prevents axes limits from being unnecessarily resized Apr 18, 2023

github-actions Bot reviewed Apr 18, 2023

View reviewed changes

QuLogic reviewed Apr 18, 2023

View reviewed changes

jklymak changed the title ~~Fixes #25682 - prevents axes limits from being unnecessarily resized~~ Prevents axes limits from being resized by axes.fill_between Apr 19, 2023

Added unit test to verify axes limits in Axes.fill_between

863502e

updated to check transform branches

c2f84c0

QuLogic approved these changes Apr 25, 2023

View reviewed changes

Comment thread lib/matplotlib/axes/_axes.py Resolved

olinjohnson requested a review from ksunden April 28, 2023 01:40

Update lib/matplotlib/axes/_axes.py

8c5305f

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

ksunden approved these changes Apr 28, 2023

View reviewed changes

ksunden merged commit 6be5be8 into matplotlib:main Apr 28, 2023

QuLogic added this to the v3.8.0 milestone Apr 28, 2023

Navigation Menu

Repository navigation

Prevents axes limits from being resized by axes.fill_between - #25712

Prevents axes limits from being resized by axes.fill_between#25712
ksunden merged 4 commits into
matplotlib:mainfrom
olinjohnson:fix-fill_between

olinjohnson commented Apr 18, 2023 •

edited by jklymak

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

ksunden commented Apr 18, 2023

Uh oh!

QuLogic Apr 18, 2023

Uh oh!

ksunden Apr 18, 2023

Uh oh!

olinjohnson Apr 20, 2023 •

edited

Loading

Uh oh!

QuLogic Apr 21, 2023

Uh oh!

olinjohnson Apr 23, 2023

Uh oh!

olinjohnson commented Apr 20, 2023 •

edited

Loading

Uh oh!

ksunden commented Apr 21, 2023

Uh oh!

olinjohnson commented Apr 21, 2023

Uh oh!

Uh oh!

ksunden commented Apr 28, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if "transform" not in kwargs:
		self.update_datalim(pts, updatex=True, updatey=True)

	if "transform" in kwargs:
	# if a transform is passed (i.e. line points not in data space),
	# data limits should not be adjusted.
	datalim = []

Conversation

olinjohnson commented Apr 18, 2023 • edited by jklymak Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ksunden commented Apr 18, 2023

Uh oh!

QuLogic Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

ksunden Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

olinjohnson Apr 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuLogic Apr 21, 2023

Choose a reason for hiding this comment

Uh oh!

olinjohnson Apr 23, 2023

Choose a reason for hiding this comment

Uh oh!

olinjohnson commented Apr 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ksunden commented Apr 21, 2023

Uh oh!

olinjohnson commented Apr 21, 2023

Uh oh!

Uh oh!

ksunden commented Apr 28, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

olinjohnson commented Apr 18, 2023 •

edited by jklymak

Loading

olinjohnson Apr 20, 2023 •

edited

Loading

olinjohnson commented Apr 20, 2023 •

edited

Loading