NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

ENH: add two new modes for 'adjustable' option#8700

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

astrofrog wants to merge4 commits intomatplotlib:masterfromastrofrog:adjustable-xlim-ylim

Closed

ENH: add two new modes for 'adjustable' option#8700

astrofrog wants to merge4 commits intomatplotlib:masterfromastrofrog:adjustable-xlim-ylim

Conversation

Copy link

Contributor

astrofrog commentedJun 1, 2017•
edited by tacaswell
Loading

PR Summary

This adds two new modes for theadjustable= option (inset_aspect and in the initializer for axes). The new modes are'xlim' and'ylim' - these behave like the 'datalim' option except that only the x or y limits are adjusted, in a deterministic way. The changes are backward-compatible. It looked like there were no (?) tests foradjustable='datalim', so I've covered that in the test I added.

Question: I need to add an entry to 'what's new', but it looks likedoc/users/whats_new.rst refers to 2.0 not 2.1?

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/whats_new.rst if major new feature
~~Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way~~

(I wasn't sure if I was supposed to tick the above, or the reviewers, maybe this could be made clearer?)

cc@tacaswell - as discussed on Gitter

ENH: add two new modes for 'adjustable' option

2fd2dd0

The new modes are 'xlim' and 'ylim' - these behave like the 'datalim' option except that only the x or y limits are adjusted, in a deterministic way.

astrofrog force-pushed theadjustable-xlim-ylim branch from1918f00 to2fd2dd0Compare

June 1, 2017 15:25

Copy link

Member

tacaswell commentedJun 1, 2017

Can you add the whats_new text tohttps://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new ? We do each one in a its own file to avoid needless conflicts. Eventually those will all be rolled up onto the top-level file.

Added 'What's new' entry

4a15e9a

Copy link

ContributorAuthor

astrofrog commentedJun 1, 2017

@tacaswell - thanks for clarifying this, I've done it now

Fix PEP8 failures

5365621

tacaswell added this to the2.1 (next point release) milestone

Jun 1, 2017

tacaswell mentioned this pull request

Jun 5, 2017

BUG: raise ValueError if sharex, sharey point to a different figure#6463

Closed

Copy link

ContributorAuthor

astrofrog commentedJun 9, 2017

@tacaswell - is there anything I can do on my side to help with this PR?

tacaswell reviewed

Jun 9, 2017

View reviewed changes

lib/matplotlib/axes/_base.py Outdated

		@@ -438,7 +438,8 @@ def __init__(self, fig, rect,
		================ =========================================
		Keyword Description
		================ =========================================
		adjustable [ 'box' \| 'datalim' \| 'box-forced']
		adjustable [ 'box' \| 'datalim' \| 'xlim' \| 'ylim' \|

Copy link

Member

tacaswellJun 9, 2017

Choose a reason for hiding this comment

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

this has to be one line or have a trailing\ because we still have some home-grown docstring parsing.

Copy link

ContributorAuthor

astrofrogJun 10, 2017

Choose a reason for hiding this comment

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

I tried on one line but got a PEP8 error in the CI, so adding a\

tacaswell approved these changes

Jun 9, 2017

View reviewed changes

Copy link

Member

tacaswell left a comment

Choose a reason for hiding this comment

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

Modulo minor docstring formatting issue.

Add trailing \ in docstring

c37354c

Copy link

Member

tacaswell commentedJun 12, 2017

There are weird errors on both travis and appveyor:

.......ssss.....ssss..s..s....s..ss...s...ssssss..s....s..s.s...s..sss.s..s.ssss..ssss..ssss..ssss..ssss.s....s.ss.s.....sss.ss....s.sss.s...s.ss.s.s.ss..s.sss.ss..ssss.s....s.sss...s...ss..sss.s..s...ssss.s.s.s..s...s..s.................s.......s.s..........s..........s.....s....s.s..s.s..ss.s.s.ss.s..sss..s..s.sss..s...s..sss.s..s.s.ss....s.....ss..................................................................................................................................s.....s.........................................s............sxsCoverage.py warning: No data was collected. (no-data-collected)INTERNALERROR> Traceback (most recent call last):INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\main.py", line 105, in wrap_sessionINTERNALERROR>     session.exitstatus = doit(config, session) or 0INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\main.py", line 141, in _mainINTERNALERROR>     config.hook.pytest_runtestloop(session=session)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 745, in __call__INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 339, in _hookexecINTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 334, in <lambda>INTERNALERROR>     _MultiCall(methods, kwargs, hook.spec_opts).execute()INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 613, in executeINTERNALERROR>     return _wrapped_call(hook_impl.function(*args), self.execute)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 250, in _wrapped_callINTERNALERROR>     wrap_controller.send(call_outcome)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 280, in get_resultINTERNALERROR>     _reraise(*ex)  # noqaINTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 265, in __init__INTERNALERROR>     self.result = func()INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\vendored_packages\pluggy.py", line 614, in executeINTERNALERROR>     res = hook_impl.function(*args)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\xdist\dsession.py", line 539, in pytest_runtestloopINTERNALERROR>     self.loop_once()INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\xdist\dsession.py", line 558, in loop_onceINTERNALERROR>     call(**kwargs)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\xdist\dsession.py", line 664, in slave_testreportINTERNALERROR>     self.sched.mark_test_complete(node, rep.item_index, rep.duration)INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\xdist\dsession.py", line 280, in mark_test_completeINTERNALERROR>     self.node2pending[node].remove(item_index)INTERNALERROR> ValueError: list.remove(x): x not in list

Copy link

Member

efiring commentedJun 12, 2017

Please hold off on this. I am almost ready to submit a PR that involves quite a few changes related to this. It will be easier to add yours on top of mine than the reverse.

Copy link

ContributorAuthor

astrofrog commentedJun 12, 2017

@efiring - sounds good

@tacaswell - are the CI failures likely related to the backslash I added? The CI passed previously before that change.

Copy link

Member

dstansby commentedJun 15, 2017

Fairly sure the CI stuff is due to a numpy 1.13 bug that is now fixed, restarting the builds to see what happens.

Copy link

ContributorAuthor

astrofrog commentedJul 25, 2017

@efiring - what is the status of the other PR? (which one is it?)

I'd really like to get these changes in 2.1 if possible (will rebase once I get the go-ahead)

Copy link

Member

tacaswell commentedSep 1, 2017

Sorry, looks like this is going to miss 2.1 😞

Hopefully 2.2 will be Nov-ish.

tacaswell modified the milestones:2.2 (next next feature release),2.1 (next point release)

Sep 1, 2017

Copy link

ContributorAuthor

astrofrog commentedSep 1, 2017

@tacaswell - I can rebase this if you still want to squeeze it in to 2.1?

Copy link

Member

efiring commentedSep 1, 2017

I would like to get#8752, or something along those lines, straightened out and merged before adding new options related to aspect-ratio handling.

jklymak added the status: needs rebase label

Jul 16, 2019

jklymak assignedjklymak andefiring and unassignedjklymak

Jul 16, 2019

jklymak added the stale label

Jul 16, 2019

Copy link

Member

jklymak commentedJul 16, 2019

@astrofrog is this 2-year-old PR something you'd still like to get in? Feel free to clear the stale flag...

Copy link

Member

efiring commentedJul 16, 2019

I'm sorry for the long holdup.#10033, which seems to be the relevant part of#8752 (now closed), is in, so if this is to go forward, now would be a reasonable time for a rebase.@astrofrog, I'm curious: what is the use case motivating this?

Copy link

Member

efiring commentedJul 17, 2019

Unfortunately,#14727 is also related, and I expect it to be merged soon, so the rebase here probably should wait for that.
I have to admit that I'm also a little uncomfortable with making the already complicated datalim code even more complicated by adding these two variations.