subplot docstring improvement (re #300) #953
Conversation
|
"keep_super_axes" is new functionality and should be noted in the API Changes. Also, what is the motivation for this feature? |
| @@ -746,31 +763,36 @@ def subplot(*args, **kwargs): | |||
| for the subplot. This projection must have been previously | |||
| registered. See :mod:`matplotlib.projections`. | |||
|
|
|||
| *keep_super_axes*: | |||
| A boolean flag indication whether the removal of axes which | |||
| fully overlap the newly created one should take place. | |||
There was a problem hiding this comment.
This sentence is a bit awkward. How about:
"When True, remove axes which fully overlap the newly created axes. Default is False."
There was a problem hiding this comment.
Proof that the sentence was awkward...you got it the wrong way around.
"When False, remove axes which fully overlap the newly created axes. Default is False."
or
"When True, keep any axes which fully overlap the newly created axes. Default is False."
or
I remove the whole thing...
|
The documentation changes look good. The keep_super_axes feature should probably have its own pull request and a note in the API changes, as @WeatherGod said. I think it would also be helpful to have @leejjoon comment on that feature , as one who has dealt a lot with axes management. |
Good question. I don't need it, but it is/was documented as saying: I either had the choice of removing the statement completely, or implementing something (if users will never have a need for |
| plot(rand(12), rand(12)) | ||
| subplot(212, axisbg='y') # creates 2nd subplot with yellow background | ||
|
|
||
| This functionality can be disabled by setting *keep_super_axes* to be |
There was a problem hiding this comment.
I'm happy to remove the new keyword, but I do feel uncomfortable about not being able to make a simple statement about how to disable this behaviour. Maybe, given that mpl has been around so long, such functionality really isn't needed, in which case, I could simply remove this line...
|
At the very least, we are going to need this branch rebased. |
|
I rebased (looks like it needs doing again). If anybody is willing to pick it up, then assign it to yourself and put it in the 1.2.x milestone and I will rebase and polish. Otherwise, I think it is going to miss the freeze. |
|
It would be nice to get the docstring part of this change in, even if the code part doesn't. I think docstring-only changes are generally ok during the freeze anyway. |
|
Ok. Fair plan. |
|
Is it worth getting this into 1.2? As @mdboom points out, getting clearer docstrings in, in my opinion, will only help our users. |
…ailing list thread "BUG: RuntimeError: dictionary changed size during iteration"
that xerr and yerr are always relative to the data location.
slight tweak to the documentation of `errorbar`
Fix bug updating WeakKeyDictionary during iteration
Fixed a variable name typo
Update examples/pylab_examples/histogram_demo_extended.py
|
|
||
| .. note:: | ||
|
|
||
| Creating a new subplot with a position which is fully consumed by a |
There was a problem hiding this comment.
"fully consumed by" -> "entirely inside"
|
@pelson, I think that if you apply the docstring changes only, so you are changing only pyplot.py, then they can be merged quickly and the PR backlog will be reduced by one. I would suggest targeting master. (I suggested one wording change inline.) As for a follow-up PR, I'm not sure what to do about "keep_super_axes". I don't like the kwarg name, and I don't like the existing mis-feature (auto-deletion of an existing containing Axes). It adds much more complexity, and documentation requirement, than it could possibly be worth. Adding the ability to defeat it with a kwarg is the usual cumbersome way of moving to deprecate and remove it. I think I would support adding the kwarg only if this is its goal, so that it will eventually go away. I don't know what to call it. Maybe "remove_enclosing", with default starting as True, changing to False, and then vanishing, to the cheers of the crowd. |
|
I've done the work, but I've based it against v1.2.x - I don't have enough github foo to change the target of this PR (I'm not sure that is even possible with v3 of github's api), so will close and create a new PR. |
I have tried to improve the documentation of the
subplotfunction, I don't expect this to be perfect, but I do think its an improvement.This PR is in response to issue #300