-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Conversation
68d02b4
to
b15fb4e
Compare
This looks slightly more lengthy than this one; I wonder if I was missing something (other than infinite slopes). |
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. |
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? |
Oh, sorry, I saw the accepted solution in the SO thread which just uses xlim/ylim.
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') |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
discussed during the call:
|
Looks good. |
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. |
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.
Anybody can merge after CI pass. |
🎉 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]) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 meetorigin + 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
toaxline
to have it skip this step.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
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 |
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. |
There was a discussion on xy1, xy2 vs. intercept, slope, but I cannot find the related issue. AFAIR the arguments were:
So it should only be one of the two. Additionally,
Additionally, vertical lines (infinite slope) would need special handling. |
Are |
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. |
OK. I don't really understand the motivation for the decision, but it's your library... |
@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 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 |
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. |
But again, those are rarely described by the user as two points. |
Agreed, but I don't know of a better parametrization which actually works in all three cases. |
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 Another suggestion: I agree that having both parameterizations in one function is confusing. But the signature to
Such that when you have slope
Perhaps it's annoying enough to feel like matplotlib, but useful enough to make most people happy. |
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. |
I'm actually considering if a generalization of Small extension
With the private Valid usages:
For the slope, intercept case, this simplifies at least to Theoretically, one could go as far as also supporting intercept directly as an additional kw-only arg. Larger extension
Valid usages:
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. |
Your post is going back and forth between Edit: Sorry for the confusion. It should all be |
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). |
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.
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
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 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. |
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. |
Reading through the discussion here, I wonder if what people want might be a general purpose function plotter, something like
which would evaluate |
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. |
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