ENH: first draft of ensure_ax decorator by tacaswell · Pull Request #4488 · matplotlib/matplotlib · GitHub

Decorator for making user functions using the suggested signature
work with the state machine to plot the the implicit currently active
Axes.

This started from a discussion at lmfit/lmfit-py#146 (comment) and was mostly implemented by @danielballan at https://github.com/Nikea/xray-vision/blob/master/xray_vision/utils/mpl_helpers.py (which is for my day-job at BNL).

This is roughly related to my on-going attempts to simplify pyplot (see #3587, #2736 and #4091)

This need a lot more documentation before they are merged, throwing this up to get feed back.

@matplotlib/developers Thoughts on this?

I would like to present some version of this in my scipy talk so would like feed back as soon as possible.

I haven't tested it yet, but it looks good.
Maybe the decorators could also add some boilerplate documentation to cover the change in signature options? I don't know how hard this would be.

@efiring Thoughts on docstring changes?

@tacaswell, the docstring change looks good.

command-line use? Might be misleading. Would probably be better as just "interactive use".

I want to keep something in there so we don't encourage people to use these
functions in other functions.

I am a bit hesitant to include this at all for exactly that reason.

On Tue, Jul 7, 2015, 3:06 PM Benjamin Root notifications@github.com wrote:

In lib/matplotlib/pyplot.py
#4488 (comment):
   else:
       ax = gca()
   return func(s, ax, _args, *_kwargs)
pre_doc = inner.doc

if pre_doc is None:
   pre_doc = ''
else:
   pre_doc = dedent(pre_doc)
inner.doc = _ENSURE_AX_DOC + pre_doc

return inner
+def register_func(func):

"""

Registers a function to be wrapped and made available in the

pyplot namespace for convenient interactive command line use.
command-line use? Might be misleading. Would probably be better as just
"interactive use".

—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/4488/files#r34083841.

@tacaswell Does this need an example? At least I feel a bit confused by it as I don't see it used anywhere (and I haven't learnt about decorators yet)...

Get rid of the None. If ax is (somehow?) set to None here the user function that expects an axes will explode

Well it looks meaningless anyhow as we have checked that we have 'ax' as a keyword argument, thus it will never use the default...

Good catch on that, I think I changed how this logic was going to be handled half way through writing it.

Looks good. I doubt this is the case, but just to anticipate a question from the peanut gallery: does this add significant overhead vs. the current approach?

One other question, what does ENH stand for? (Also I have seen MNT and the likes in PR titles).

ENH -- Enhancement
MNT -- Maintenance
WIP -- Work In Progress

On Wed, Jul 8, 2015 at 11:33 AM, OceanWolf notifications@github.com wrote:

One other question, what does ENH stand for? (Also I have seen MNT and the
likes in PR titles).

—
Reply to this email directly or view it on GitHub
#4488 (comment)
.

I'm assuming it's
MNT: maintenance, cleanups, refactoring
FIX: bugfix
ENH: enhancement, new feature
I don't know if it's official policy, but I imagine it can be useful when dealing with large numbers of pull requests.
On 8 Jul 2015 6:33 pm, OceanWolf notifications@github.com wrote:One other question, what does ENH stand for? (Also I have seen MNT and the likes in PR titles).

—Reply to this email directly or view it on GitHub.

This is shameless stolen from numpy, there is a key in their development
docs (probably should copy to ours).

On Wed, Jul 8, 2015, 11:24 AM Jouni K. Seppänen notifications@github.com
wrote:

I'm assuming it's
MNT: maintenance, cleanups, refactoring
FIX: bugfix
ENH: enhancement, new feature
I don't know if it's official policy, but I imagine it can be useful when
dealing with large numbers of pull requests.
On 8 Jul 2015 6:33 pm, OceanWolf notifications@github.com wrote:One
other question, what does ENH stand for? (Also I have seen MNT and the
likes in PR titles).

—Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub
#4488 (comment)
.

that key is a little below this link.

Maybe replace each of these "func" strings with "{0}" so the actual name can be interpolated. See below.

@njsmith expressed grave concern about the registration function, please don't merge this until that gets sorted (I think I am leaning towards agreeing with him that this is a bad idea).

@ericdill Added one of the decorators you wanted 🍺

I think this is ready to merge.

The addition of quick_axes should probably get some more eyes on it.

Shouldn't the new information be at the end? This way it even overwrites the first line?

I put it at the top because it is doing semi-scary things to the signature (moving a required arg to the end). Is there a standard way of doing this when using partials?

Internally we do not do a good job of having a meaningful first line.

In phone conversation with @efiring he suggesting changing 'quick_axes' to `gna` for 'get new axes' in analogy and contrast to `gca` -> 'get current axes (and create one if you have to)'.

rebased to fix import conflict in pyplot

I owe the mailing list several MEPs.

👍 - you've much more time thinking about improvements to a very old codebase than anybody else, and you've got some great ideas - I think for the sake of the community it is well worth writing small MEPs which we can all buy-in to.

For that reason, I don't believe this is a release critical PR, and I wouldn't want to see a release held up for it. Maybe we need a "a core dev is advocating this for the next release, and we should do our best to make sure it gets in" milestone 😄

I see a lot of these "Release critical" things as extra features that might help a few people... I don't understand what makes something "Release critical". In debian, they have a strict set of goals (MEPs) that they want to achieve for the next release, like ensuring everything supports utf8...

👍 to needing more MEPs, but I don't mind them large, or at least overarching... we can always split up MEPs into smaller sub MEPs and those into even smaller PRs... for instance we have a large backend refactor now and that got divided into two sub-MEPs, MEP22 and MEP27, and those get broken down into smaller PRs.

One thing we do need before that though, a way of sorting out the MEPs, I like that we now submit MEPs through PRs, but... we now have a problem with numbering, if we have multiple MEPs sitting in PRs not yet merged, it becomes a nightmare to keep track of numbers, and we will have merge conflicts, etcetera, so we need to sort that out...

Okay, I have just started writing an MEP on an axes refactor/overhaul, written from the ground up. Depending on whether we achieve full Backwards Compatibility with the MEP looks by the end I guess will determine whether this will go out in 2.x or 3.0.

But first to figure out how to do MEPs based on my points above...

I expected this to be a relatively non-controversial PR.

@OceanWolf With the exception of the labeled data unpacking (which is a release critical feature, we need to address the reality of labeled data being wide-spread) all of these feature have been in the milestone for several months. To be fair, they are there because I put them there, but no one argued at the time and as I am running the release I am going to claim that as my prerogative 😉. The main thing that makes these 'critical' is that they are new features (which can't be merged once we have an RC), not bug fixes (which can be).

@pelson I am not sure what your hesitation about the decorators. I am happy to mark these as experimental if that makes you more comfortable. The logic on how I got to them was basically

I want people to write functions like def plotter(ax, data, style) which can assert that it gets an explicit axes (and does not need to even know that pyplot exists). This is good for re-usability / composability etc. We also have a large number of functions with this signature (the Axes methods) that we can work with internally. I think that many people should be writing many more of these functions that can take in rich data objects for domain-specific plotting tools.
people complain about having to use explicit axes (I don't find these complaints particularly compelling, but enough people have independently said this that I believe our users about it) so if we don't want to have every user write their own (slightly different!) way of handling the axes, we should provide a decorator to do that. A obvious thing to do would be to have the decorated function just take an ax kwarg, but then the resulting function no longer has the same signature (which is bad from a consistency point of view, arts = fun(ax, data, style) should always work, from switching between interactive/non-interactive work (as implicit axes should only really be used at the repl)). Hence the crazy first arg introspection to sort out if it is an axes or not.
a bunch of down-stream libraries already use the ax as a kwarg pattern so we should support that too

The weakest point that I see in there is my dislike of breaking ax as first arg always working.

Long term I think the list of figure/axes creation methods in pyplot should go down to:

gcf / gnf (get current / new figure)
gca / gna (get current / new axes)
subplots(...) (make a grid of axes)

and everything else should be deprecated.

I share Tom's surprise that this PR is arousing discomfort. But evidently it is, so we need some clear discussion as to why, and what to do about it. I think that Tom's rationale for it is quite clear. I am open to arguments against it, or to suggested modifications or alternatives.

On 2015/09/03 7:51 PM, Thomas A Caswell wrote:

The weakest point that I see in there is my dislike of breaking |ax| as
first |arg| always working.

Sorry, I haven't been able to figure out what you mean here. Are some
words missing?

The suggested signature for the non implicit calls is func(ax, data,
style). We could have the decorated function have the signature func(data,
style, ax=ax) vs allowing the decorated function to also be used as
func(ax, data, style). The logic to use the wrapped function with both
signature is where most of the ugly code comes from.

The trade off is partially consistent api between interactive/non
interactive vs code complexity in the decorator.

On Fri, Sep 4, 2015, 02:22 Eric Firing notifications@github.com wrote:

On 2015/09/03 7:51 PM, Thomas A Caswell wrote:

The weakest point that I see in there is my dislike of breaking |ax| as
first |arg| always working.

Sorry, I haven't been able to figure out what you mean here. Are some
words missing?

—
Reply to this email directly or view it on GitHub
#4488 (comment)
.

I share Tom's surprise that this PR is arousing discomfort.

I guess the concern comes down to the fact that right now this PR doesn't make the mpl codebase more succinct and it represents 4 new functions in the pyplot namespace which need consideration.

If we want to iterate on this in this PR, rather than as a MEP, I'm happy to do that, but I suspect there will be several threads to the discussion which doesn't read well in a linear GH style...

I'll outline some of my concerns, and we can go from there:

ensure_ax.* decorators

It is unclear to me that a decorator is the way to go here:

It isn't until py3.4 that functools.wraps preserves the wrapped fn's signature, so we end up with even more *args, **kwargs style documentation.
Whilst it is technically possible to do, it is plain weird (and therefore unexpected) to define a positional argument which is optional. There is no reconciliation of this which puts axes as the first argument of a plotting function at the same time as making it optional.

Assuming the weird thing is a deal breaker, the problem is hugely simplified, and a function would do:

def my_plotting_function(arg1, arg2, ax=None):
    ax = plt.ensure_axes(ax)

Truthfully though, the backwards compatible way is just as good:

def my_plotting_function(arg1, arg2, ax=None):
    ax = ax if ax is not None else plt.gca(ax)

The question about whether axes should be the first argument for all plotting functions needs to be also addressed. I completely understand the recommendation, but doing so means that you cannot then call the argument optional (without breaking the expected Python definition of positional). So I think we need to look at the motivation again - is the recommendation that axes be the first argument related to the fact that our current Axes methods are implicitly doing this (through self)? Or is there another reason why accepting ax as an optional argument is an unhealthy recommendation? The idea that developers wouldn't need pyplot (specifically gca) in their functions is hampered by the fact that these wrappers themselves live in pyplot.

Finally, if there are to be this many repetitively named items in pyplot, I'd consider giving them a namespace (via a class) - it would also allow for some of the duplicated code to be shared between functions.

gna.* function

I'm all in favour of improving the interface - plt.figure().gca() is hardly a pleasant alternative. It has irked me for a long time that gca actually returns a subplot object though - I have never seen the downside of a Axes (rather than a Subplot), and so quite happily use plt.axes rather than plt.subplots (which obviously do very different things). My point being that gna continues this trend of using a single Subplot, rather than an Axes.

I appreciate we are where we are, but the gc* family of functions have never been appealingly named, and given we are talking seriously about making improved interfaces I think this is an opportunity to think about how we want modern matplotlib to look.

Incidentally, I was surprised that only those two kwargs were added to the gna implementation, and that gna is making a new figure (I would have expected it to use an existing figure, but then where would it put that axes).

I hope this isn't coming across as negative - that is not my intention. I think there is room for debate here about the best course for us to be setting for matplotlib, and I'm very comfortable with @tacaswell leading that discussion; I just want to ensure we have it rather than take ad-hoc changes without an overarching strategy.

On 2015/09/04 8:08 AM, Phil Elson wrote:

I share Tom's surprise that this PR is arousing discomfort.
I guess the concern comes down to the fact that right now this PR
doesn't make the mpl codebase more succinct and it represents 4 new
functions in the pyplot namespace which need consideration.

If we want to iterate on this in this PR, rather than as a MEP, I'm
happy to do that, but I suspect there will be several threads to the
discussion which doesn't read well in a linear GH style...

MEPs via GH have the same problem, don't they? Should we move this to
the mailing list?

I'll outline some of my concerns, and we can go from there:

Very helpful, thank you.

@tacaswell I understand that we can't merge these features between RC and 1.5, but that doesn't prevent us from releasing this in 2.1? I would rather see us get this right, than rush it.

👍 to what @pelson says regarding an overarching strategy and about the naming, I really dislike gcf, etcetera. it took me ages to realise what this meant having seen it many times sprawled throughout the code I saw it sprawled throughout the code... lets keep it readable.

@efiring I think MEPs allow us to more easily discuss logic, by commenting on specific ideas/lines. Coincidently I have just pushed an MEP (#5029) to begin a conversation around the future of the axes class. Quite wide-ranging, but I don't think it will cover this area specifically, but it may pick up on a few of the points said here and add to the list of questions pertaining to this PR...

Block quoting to make it clear what I am responding to, the gna discussion is simpler.

gna.* function

I'm all in favour of improving the interface - plt.figure().gca() is hardly a pleasant alternative. It has irked me for a long time that gca actually returns a subplot object though - I have never seen the downside of a Axes (rather than a Subplot), and so quite happily use plt.axes rather than plt.subplots (which obviously do very different things). My point being that gna continues this trend of using a single Subplot, rather than an Axes.

I appreciate we are where we are, but the gc* family of functions have never been appealingly named, and given we are talking seriously about making improved interfaces I think this is an opportunity to think about how we want modern matplotlib to look.

Incidentally, I was surprised that only those two kwargs were added to the gna implementation, and that gna is making a new figure (I would have expected it to use an existing figure, but then where would it put that axes).

I didn't know the plt.axes existed (so my list way up thread should be at least one longer) and it can return either an Axes or a AxesSubplot depending on how you call it.

The point of gna is to safely (that is without messing up any existing figures) get a new axes that can be passed into plotting functions. The two kwargs I implemented are based on SO questions I have seen go by, my sense is that those two are the most frequently used. gna was implemented as a stand-alone function to support ensure_new_ax which implements the logic we discussed with Luke the last night of scipy.

Throwing it in as a top-level function seemed like a useful thing to do but I have no problem nuking it if we want to hold off on adding it for a more comprehensive simplification of this class of things (but I think it adds a useful functionality and does not make things that much worse).

As excited as I am about this, I am leaning towards giving up on it for now 😞

I really want this to go in, but not over @pelson 's protests and I want to get 1.5rc1 tagged (maybe tomorrow night?)

After phone call with @pelson we have decided to spin this off into it's own project. This lets us:

iterate faster
not pollute the pyplot namespace with possible ill-considered / thrashy API
allows us to retro-fit existing 1.3 / 1.4 installations with this functionality

👍

Thanks to @tacaswell for being very gracious on this. I think the distinct package approach gives us some great opportunities, with very little cost (an extra installation). The big selling point for me was the ability for package developers to use these wrappers for older versions of matplotlib.

👍 to @tacaswell. 👏

tacaswell added the status: needs review label Jun 2, 2015

tacaswell added this to the proposed next point release milestone Jun 2, 2015

tacaswell added the New feature label Jun 2, 2015

tacaswell changed the title ENH/WIP: first draft of ensure_ax ENH/WIP: first draft of ensure_ax decorator Jun 2, 2015

tacaswell mentioned this pull request Jun 7, 2015

Enh mappable remapper #4490

Closed

tacaswell changed the title ENH/WIP: first draft of ensure_ax decorator ENH: first draft of ensure_ax decorator Jul 7, 2015

tacaswell modified the milestones: next point release, proposed next point release Jul 7, 2015

WeatherGod reviewed Jul 7, 2015
View reviewed changes

ericdill reviewed Jul 7, 2015
View reviewed changes

efiring reviewed Jul 11, 2015
View reviewed changes

jankatins reviewed Jul 30, 2015
View reviewed changes

tacaswell added 3 commits August 30, 2015 17:28

DOC: add whats_new entry

f9ace37

MNT: put wrapping doc at end of docstring

c44b7dd

API: rename quick_axes -> gna

e398ad8

In phone conversation with @efiring he suggesting changing 'quick_axes' to `gna` for 'get new axes' in analogy and contrast to `gca` -> 'get current axes (and create one if you have to)'.

tacaswell force-pushed the enh_pyplot_ensure_axes branch from a0bb42b to e398ad8 Compare August 30, 2015 21:28

tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Sep 2, 2015

tacaswell mentioned this pull request Sep 2, 2015

outstanding issues from jpl #4897

Closed

14 of 17 tasks

tacaswell removed the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Sep 13, 2015

tacaswell modified the milestones: unassigned, next point release Sep 14, 2015

pelson closed this Sep 14, 2015

tacaswell removed the status: needs review label Sep 14, 2015

tacaswell mentioned this pull request Oct 27, 2016

plt.hist does not obey the hist.bins rcparams #7356

Closed

tacaswell mentioned this pull request Aug 31, 2017

ENH: add ax kwarg to every pyplot function #9111

Closed

3 of 5 tasks

tacaswell mentioned this pull request Apr 26, 2019

RFC: new function-based API #14058

Closed

ENH: first draft of ensure_ax decorator by tacaswell · Pull Request #4488 · matplotlib/matplotlib · GitHub

Repository navigation

ENH: first draft of ensure_ax decorator - #4488

ENH: first draft of ensure_ax decorator#4488tacaswell wants to merge 11 commits intomatplotlib:mastermatplotlib/matplotlib:masterfrom tacaswell:enh_pyplot_ensure_axestacaswell/matplotlib:enh_pyplot_ensure_axesCopy head branch name to clipboard

Conversation

tacaswell commented Jun 2, 2015

Uh oh!

tacaswell commented Jun 7, 2015

Uh oh!

efiring commented Jun 7, 2015

Uh oh!

tacaswell commented Jul 7, 2015

Uh oh!

efiring commented Jul 7, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OceanWolf commented Jul 7, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdboom commented Jul 8, 2015

Uh oh!

OceanWolf commented Jul 8, 2015

Uh oh!

WeatherGod commented Jul 8, 2015

Uh oh!

jkseppan commented Jul 8, 2015

Uh oh!

tacaswell commented Jul 8, 2015

Uh oh!

ericdill commented Jul 11, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Jul 11, 2015

Uh oh!

tacaswell commented Jul 16, 2015

Uh oh!

tacaswell commented Jul 30, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Aug 30, 2015

Uh oh!

pelson commented Sep 3, 2015

Uh oh!

OceanWolf commented Sep 3, 2015

Uh oh!

OceanWolf commented Sep 3, 2015

Uh oh!

tacaswell commented Sep 4, 2015

Uh oh!

efiring commented Sep 4, 2015

Uh oh!

efiring commented Sep 4, 2015

Uh oh!

tacaswell commented Sep 4, 2015

Uh oh!

pelson commented Sep 4, 2015

ensure_ax.* decorators

gna.* function

Uh oh!

efiring commented Sep 4, 2015

Uh oh!

OceanWolf commented Sep 5, 2015

Uh oh!

tacaswell commented Sep 5, 2015

Uh oh!

tacaswell commented Sep 13, 2015

Uh oh!

tacaswell commented Sep 14, 2015

ENH: first draft of ensure_ax decorator#4488
tacaswell wants to merge 11 commits into
matplotlib:masterfrom
tacaswell:enh_pyplot_ensure_axes