Skip to content

Add axes method for drawing infinite lines. #15330

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 23, 2019
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Sep 23, 2019

PR Summary

See #7506 (@QuLogic), #9321 (@dstansby) for earlier PRs. Fixes #5253.
This PR differs from the earlier ones by using a custom Line2D subclass which allows recomputing the transform at draw time. This is slightly inelegant but is the only solution I could think of that works on nonlinear scales as well. (This also fixes the incorrect datalimits pointed out in #9321 (comment) although that specific part could be extracted out -- see comment about not using add_line().)

Note: Here I chose to let axline update the axis limits to include xy1, xy2 in the axis limits -- I think this is consistent with axvline, axhline -- but we could also choose to make axlines just never touch the axis limits, and leave them to be whatever the other artists want.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer force-pushed the axline branch 2 times, most recently from 68d02b4 to b15fb4e Compare September 23, 2019 15:32
@ImportanceOfBeingErnest
Copy link
Member

This looks slightly more lengthy than this one; I wonder if I was missing something (other than infinite slopes).

@anntzer
Copy link
Contributor Author

anntzer commented Sep 23, 2019

This PR draws a line with "infinite" length, similarly to axvline and axhline: even if you manually (or programatically) zoom out, the line will still go from one edge of the axes to another, rather than being limited to the original view limits.

@ImportanceOfBeingErnest
Copy link
Member

Sure, I understand that. And I compared it to a previous solution to this, which does not calculate as much stuff as this one, and now I am wondering why yours is so much longer. One obvious thing is that this one here takes two points instead of a slope and hence allows infinite slopes - is this what makes it complicated?

@anntzer
Copy link
Contributor Author

anntzer commented Sep 23, 2019

Oh, sorry, I saw the accepted solution in the SO thread which just uses xlim/ylim.
Regarding your solution it looks like it suffers from similar problems as #9321, namely

  • if the infinite line is the only thing drawn, axes limits still go from 0 to 1 in each direction (which may not even show any of the plotted line), but if you add some other artist to the plot (e.g. ax.plot([5, 6], [7, 8]) -- remove the scatter() in your example) then the autolimits encompass both the additional artist and the x=0-1, y=whatever range; in other words x=0-1 is "leaking" into the other artists limits.
  • afaict it does not work for log-log scales.

Fixing these is what makes this solution so complex (plus a dozen of lines for handling the fully vertical/horizontal case in a manner that makes it possible to update xy1/xy2 a posteriori on the artist).

--------
Draw a thick red line passing through (0, 0) and (1, 1)::

>>> axline((0, 0), (1, 1), linewidth=4, color='r')
Copy link
Member

Choose a reason for hiding this comment

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

Add an example in log space...

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually this example works fine (but demonstrates vlines/hlines which behave stupidly differently from axvline/axhline) -- the example that needs to be modified is https://matplotlib.org/gallery/subplots_axes_and_figures/axhspan_demo.html.

@anntzer
Copy link
Contributor Author

anntzer commented Oct 8, 2019

discussed during the call:

  • docs could mention that this also works in log space.
  • replace "straight line" in docstring?
  • include in hline/vline tutorial?

@efiring
Copy link
Member

efiring commented Oct 8, 2019

Looks good.

@anntzer
Copy link
Contributor Author

anntzer commented Oct 8, 2019

Improved docs a bit and added example. Admittedly an example showcasing e.g. a power-law in log-log scale would be nice, but that's a bigger endeavour.

QuLogic and others added 3 commits October 22, 2019 23:21
Clean up axline

Add axline image test

Fix test image

Add what's new

Add note about log axes

Error if trying to draw line on non-linear axes

Fix scale checking

Fix docstring interpolation

Chnage to using xy1, xy2

Fix docs and closeness checking

Raise error if points are the same

Swap axline test to image comparison
This makes it also work on non-linear scales.

Also don't use add_line directly but manually copy most of add_line, to
ensure data limits are correctly set.
@timhoffm
Copy link
Member

Anybody can merge after CI pass.

@efiring efiring merged commit 42e3f92 into matplotlib:master Oct 23, 2019
@QuLogic QuLogic added this to the v3.3.0 milestone Oct 23, 2019
@anntzer anntzer deleted the axline branch October 23, 2019 09:25
@anntzer anntzer mentioned this pull request Nov 23, 2019
6 tasks
@dstansby
Copy link
Member

🎉 at last, many thanks to everyone who has worked on this over the years(!)

line.set_label(f"_line{len(self.lines)}")
self.lines.append(line)
line._remove_method = self.lines.remove
self.update_datalim([xy1, xy2])
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to provide a way to exclude this

Copy link
Contributor

Choose a reason for hiding this comment

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

To elaborate on this comment:

I've come across two use-cases where this scaling behavior is undesirable:

  • I'm plotting a line with a single special point (eg, a ray, and I'm ok with the backwards portion of the line). If I plot (origin, origin + direction), then my axis bounds scale to meet origin + direction, even though that point is arbitrary. I can work around that with a hack like (origin, origin + eps*direction), but it forces me to trade off axis scaling against precision.

  • I'm plotting a true infinite line, with no special points. Perhaps ideally I'd get axis scaling that shows the "nearest" part of the line, but I'd be ok with just passing autoscale=False to axline to have it skip this step.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that it should be possible to skip the autoscaling. The two points being specified only need to be somewhere along the desired infinite line, and it's perfectly reasonable that they might not be within the domain one actually wants to view. I'm actually wondering whether it would make more sense to leave out the update_datalim entirely instead of adding a kwarg; but maybe it's too late for that. In any case, please open a new issue about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's move this discussion to #16264 (which I replied to already).

@mwaskom
Copy link

mwaskom commented Jan 18, 2020

I guess it might by too late ... but in many cases a (slope, intercept) parameterization would be more convenient than (xy1, xy2). Is there any appetite for expanding this function to include that? Or building on this work with a separate axabline (taking the nomenclature from R) that offers that parameterization?

@eric-wieser
Copy link
Contributor

Slope / intercept is degenerate for vertical lines, so if forced to pick one this is the right one. Origin / direction would somewhat address my comment above, but isn't very different.

@timhoffm
Copy link
Member

There was a discussion on xy1, xy2 vs. intercept, slope, but I cannot find the related issue. AFAIR the arguments were:

  • We don't want to support both signatures in one function. This would be quite messy.
  • We don't want two separate functions, one with each signature.

So it should only be one of the two. Additionally,

  • If you know slope and intercept, getting two points is easy
    p1 = 0, intercept
    p2 = 1, interept + slope
    
  • If you have two points, calculating slope and intercept is more tedious
    slope = (p1[1] - p2[1]) / (p1[1] - p2[1])
    intercept = p1[1] - slope * p1[0]
    

Additionally, vertical lines (infinite slope) would need special handling.

@mwaskom
Copy link

mwaskom commented Jan 18, 2020

Are axhline and axvline being deprecated? (Please say no...)

@timhoffm
Copy link
Member

timhoffm commented Jan 18, 2020

No. They stay. Horizontal and vertical lines are relevant enough that they can be dedicated methods. Also, deprecation would break a lot of existing code, which would not be justifiable.

@mwaskom
Copy link

mwaskom commented Jan 18, 2020

OK. I don't really understand the motivation for the decision, but it's your library...

@jklymak
Copy link
Member

jklymak commented Jan 18, 2020

@mwaskom, I agreed with you during the original discussion.

The typical use case for me is that I'd know the slope and intercept (usually from something theoretical, a paper, or if I ran polyfit) and want to plot the line. No one cites in their paper two points, they cite the slope and the intercept. polyfit doesn't return two points, it returns the slope and the intercept.

So, with this implementation, what I'd bet are 98% of the use cases make folks do a bit of math to come up with the two points, with the only gain being that they can use this function instead of axvline(x0) for the infinite slope case. As for the tedious math going the other way, 98% of the time you want to quote the result of that tedious math in your final results, and you would do so a slope, intercept.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 19, 2020

For me there are two issues with slope/intercept: they don't work for vertical lines, and more importantly they only work (effectively) for linear scales, whereas I explicitly wanted to be able to draw exponential decays in semilog scale and power laws in loglog scale.

@jklymak
Copy link
Member

jklymak commented Jan 19, 2020

But again, those are rarely described by the user as two points.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 20, 2020

Agreed, but I don't know of a better parametrization which actually works in all three cases.

@mwaskom
Copy link

mwaskom commented Jan 20, 2020

I see the logic of favoring a slightly more general function that is less convenient for the large majority of use cases, even though it is the kind of API decision that makes people think matplotlib is annoying to use. But I don't really understand the hard constraint on adding two functions. There's already going to be axline, axhline and axvline. How much marginal confusion is added by a fourth option, axabline?

Another suggestion: I agree that having both parameterizations in one function is confusing. But the signature to axline could be

axline(xy1, xy2=None, slope=None, ...)

Such that when you have slope m and intercept b, you only need write

axline((0, b), slope=m)

Perhaps it's annoying enough to feel like matplotlib, but useful enough to make most people happy.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 20, 2020

I am personally not strongly opposed to axabline() (however you want to call it), I guess just -0, someone just needs to write it (and hopefully check that it behaves sensibly (possibly erroring out) in log scale). Shoehorning this into axline() looks not so nice, though.

@timhoffm
Copy link
Member

timhoffm commented Jan 20, 2020

I'm actually considering if a generalization of axline() would be the way to go.

Small extension

def axline(xy1, xy2=None, *, slope=None, **kwargs)
    """
    Add an infinitely long straight line. 

    The line can either be defined by one point and a slope or by two points.
    """
    xy1, xy2 = _to_points(xy1, xy2, slope)

With the private _to_points() doing all the checking and calculation. This has the advantage, that you can easily draw a tangent, which is a line with a known slope going through a given point.

Valid usages:

axline(xy1, xy2)
axline(xy, slope=2)

For the slope, intercept case, this simplifies at least to axhline((0, intercept), slope=slope).

Theoretically, one could go as far as also supporting intercept directly as an additional kw-only arg.

Larger extension

def axline(xy1=None, xy2=None, *, slope=None, intercept=None, **kwargs)
    """
    Add an infinitely long straight line. 

    The line can either be defined by
   
    - two points
    - one point and a slope
    - a slope and an intercept
    """
    xy1, xy2 = _to_points(xy1, xy2, slope, intercept)
    ...

Valid usages:

axline(xy1, xy2)
axline(xy, slope=2)
axline(intercept=3, slope=2)

I'd tend to go with the first one, which seems a straight forward generalization (extending to the second one is always possible later). But I also wouldn't be be opposed going all the way to the second. The API is a bit overloaded, but as discussions show, there are multiple valid opinions/usecases.

@mwaskom
Copy link

mwaskom commented Jan 20, 2020

Your post is going back and forth between axhline and axline in a confusing way. Which function are you proposing to generalize? I think axline would be the more obvious function to overload, both because it has the more general name and because it's new. Adding functionality to axhline would be less discoverable.

Edit: Sorry for the confusion. It should all be axline - fixed above. Tim

@efiring
Copy link
Member

efiring commented Jan 20, 2020

@timhoffm I think your "Small extension" is the way to go. The "Larger extension" has no advantages from the ease-of-use and readability standpoint, and is more complicated. @mwaskom, all this relates to axline; I'm sure the axhline references are just typos.

@jklymak
Copy link
Member

jklymak commented Jan 20, 2020

I like that approach. But I think we'd need to think about what slope and intercept mean in the loglog and semilog cases. ie if the scales are loglog, slope/intercept could/should mean intercept * x^slope. In general, I'm not sure (i.e. logit scales?, etc etc).

@timhoffm
Copy link
Member

timhoffm commented Jan 26, 2020

Even when specifying two points, the meaning of a "straight line" is unclear and it's usefulness in the current implementation is questionable: The line does not represent a simple relation between the x and y data coordinates anymore.

import matplotlib.pyplot as plt
import numpy as np
x = np.linspace(1, 1000, 21)

plt.semilogx(x, x, 'o')
plt.axline((1, 1), (1000, 1000))

grafik

I propose to not support any non-linear axes and raise an error instead. If there's some request and a plausible use case, we can always add such functionality later.

Edit: I just read the docstring of axline()

This draws a straight line "on the screen", regardless of the x and y
scales, and is thus also suitable for drawing exponential decays in
semilog plots, power laws in loglog plots, etc.

While technically, that's possible, I think it's not a good API. Does anybody really need "exponential decay through two points" or "powerlaw through two points"? I suspect that these would better be separate explicit functions. Therefore I stand by my claim that axline() should only be used with linear scales.

However, if there is consensus that we should keep two-point-non-linear-scale, I would also be ok to just not allow slope, intercept for non-linear scales.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 26, 2020

Well, I wrote this explicitly with the intent of supporting exponential and loglogs, and yes, two-point is not a great parametrization of such things, but there is no canonical parametrization of such things (for exponentials, do you want to use the rate? the mean lifetime? the half-life? do you want to have an implicit minus in front of the rate, which would help those (likely the majority) plotting decays, but look weird when you're not?), so two-point is not actually that bad IMO.

@ImportanceOfBeingErnest
Copy link
Member

Reading through the discussion here, I wonder if what people want might be a general purpose function plotter, something like

 Axes.plot_function(func, xrange=None, yrange=None, in_autoscale=None, density=1)

which would evaluate func either over a given range or over the current datarange of the axes at draw time. For a line with slope and intersept, it would then be about calling
ax.plot_function(lambda x: slope*x + intercept).

@timhoffm
Copy link
Member

That would in principle be interesting, but only if it would automatically resample or directly draw the function in the backend. For fixed sampling and ranges ˋax.plot(x, f(x))ˋ already works.

Resampling or direct drawing is a much more complex topic than defining/calculating a line once and drawing it to infinity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

abline() - for drawing arbitrary lines on a plot, given specifications.
9 participants