ENH: add two new modes for 'adjustable' option by astrofrog · Pull Request #8700 · matplotlib/matplotlib

astrofrog · 2017-06-01T14:47:11Z

PR Summary

This adds two new modes for the adjustable= option (in set_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 for adjustable='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 like doc/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

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.

tacaswell · 2017-06-01T16:00:27Z

Can you add the whats_new text to https://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.

astrofrog · 2017-06-01T16:18:45Z

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

astrofrog · 2017-06-09T21:16:03Z

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

tacaswell · 2017-06-09T21:21:06Z

lib/matplotlib/axes/_base.py

          Keyword            Description
          ================   =========================================
-          *adjustable*       [ 'box' | 'datalim' | 'box-forced']
+          *adjustable*       [ 'box' | 'datalim' | 'xlim' | 'ylim' |


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

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

tacaswell

Modulo minor docstring formatting issue.

tacaswell · 2017-06-12T03:46:19Z

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_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\_pytest\main.py", line 141, in _main
INTERNALERROR>     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 _hookexec
INTERNALERROR>     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 execute
INTERNALERROR>     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_call
INTERNALERROR>     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_result
INTERNALERROR>     _reraise(*ex)  # noqa
INTERNALERROR>   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 execute
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\xdist\dsession.py", line 539, in pytest_runtestloop
INTERNALERROR>     self.loop_once()
INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\xdist\dsession.py", line 558, in loop_once
INTERNALERROR>     call(**kwargs)
INTERNALERROR>   File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\xdist\dsession.py", line 664, in slave_testreport
INTERNALERROR>     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_complete
INTERNALERROR>     self.node2pending[node].remove(item_index)
INTERNALERROR> ValueError: list.remove(x): x not in list

efiring · 2017-06-12T07:52:13Z

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.

astrofrog · 2017-06-12T11:53:34Z

@efiring - sounds good

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

dstansby · 2017-06-15T11:21:02Z

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

astrofrog · 2017-07-25T13:28:35Z

@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)

tacaswell · 2017-09-01T02:19:22Z

Sorry, looks like this is going to miss 2.1 😞

Hopefully 2.2 will be Nov-ish.

astrofrog · 2017-09-01T08:14:48Z

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

efiring · 2017-09-01T16:41:52Z

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 · 2019-07-16T19:23:17Z

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

efiring · 2019-07-16T20:02:40Z

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?

efiring · 2019-07-17T06:49:04Z

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.

jklymak · 2019-07-24T13:46:34Z

I'm uncomfortable too. I'm going to close, but @astrofrog, if you want it re-opened for more discussion that would be great!

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 the adjustable-xlim-ylim branch from 1918f00 to 2fd2dd0 Compare June 1, 2017 15:25

Added 'What's new' entry

4a15e9a

Fix PEP8 failures

5365621

tacaswell added this to the 2.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

tacaswell reviewed Jun 9, 2017

View reviewed changes

tacaswell approved these changes Jun 9, 2017

View reviewed changes

Add trailing \ in docstring

c37354c

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

jklymak added the status: needs rebase label Jul 16, 2019

jklymak assigned jklymak and efiring and unassigned jklymak Jul 16, 2019

jklymak added the stale label Jul 16, 2019

jklymak closed this Jul 24, 2019

story645 removed this from the future releases milestone Oct 6, 2022

Uh oh!

Conversation

astrofrog commented Jun 1, 2017 • edited by tacaswell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

tacaswell commented Jun 1, 2017

Uh oh!

astrofrog commented Jun 1, 2017

Uh oh!

astrofrog commented Jun 9, 2017

Uh oh!

tacaswell Jun 9, 2017

Choose a reason for hiding this comment

Uh oh!

astrofrog Jun 10, 2017

Choose a reason for hiding this comment

Uh oh!

tacaswell left a comment

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Jun 12, 2017

Uh oh!

efiring commented Jun 12, 2017

Uh oh!

astrofrog commented Jun 12, 2017

Uh oh!

dstansby commented Jun 15, 2017

Uh oh!

astrofrog commented Jul 25, 2017

Uh oh!

tacaswell commented Sep 1, 2017

Uh oh!

astrofrog commented Sep 1, 2017

Uh oh!

efiring commented Sep 1, 2017

Uh oh!

jklymak commented Jul 16, 2019

Uh oh!

efiring commented Jul 16, 2019

Uh oh!

efiring commented Jul 17, 2019

Uh oh!

jklymak commented Jul 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

astrofrog commented Jun 1, 2017 •

edited by tacaswell

Loading