| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
Sorry, something went wrong.
| ms = vect._mutation_scale | ||
| stylekw = { | ||
| "head_length": kwargs.get("head_length", 12) / ms, | ||
| "head_width": kwargs.get("head_width", 12) / ms, | ||
| "tail_width": kwargs.get("tail_width", 4) / ms, | ||
| } |
There was a problem hiding this comment.
For context this numbers are based on how annotate styles it's arrows.
Sorry, something went wrong.
| else "list[Polygon]") | ||
| return tops, bins, cbook.silent_list(patch_type, patches) | ||
|
|
||
| def vector(self, dx, dy, x=0., y=0., **kwargs): |
There was a problem hiding this comment.
| def vector(self, dx, dy, x=0., y=0., **kwargs): | |
| def vector(self, x, y, dx, dy, **kwargs): |
Sorry, something went wrong.
There was a problem hiding this comment.
This may be an peculiarity of my field, but crystallographers frequently plot arrows in the complex plane that are by default rooted at zero. As evidence that I am not alone, have a look at an image search for Argand diagram. From my perspective, x=y=0 is a sensible default. I can understand how this might not be universal.
I find vector(tail_x, tail_y, head_x, head_y, ...) is also a natural four-parameter representation. Is that one the table?
Sorry, something went wrong.
There was a problem hiding this comment.
This seems a good start, but does not, so far as I can tell, allow for arrays of x, y, dx, and dy to be passed, whereas that is a natural thing to do. In which case you probably want a PolyCollection?
You also have not taken into account our units system - x, dx etc need to be able to take datetime and other units, and needs tests for these. Maybe see errorbar (?) for an example of deltas being used with the unit system.
Sorry, something went wrong.
|
@jklymak , I don't think plt.vector needs to support arrays for the following two reasons
|
Sorry, something went wrong.
|
Indeed plt.quiver provides this functionality, so why do we need vector? It's easy enough to pass a single x, y, u, v to quiver. |
Sorry, something went wrong.
I tried this out a bit and quiver is unfortunately not good substitute. Trying to replicate vector requires two extra kwargs and still doesn't replicate it. It also doesn't follow the color cycle import matplotlib.pyplot as plt
import numpy as np
fig, ax = plt.subplots()
x = .5
y = .5
dx = 1
dy=10
ax.vector(dx, dy, x ,y, label='vector')
shift = .05
ax.quiver(x+shift,y+shift, dx, dy, angles='xy', scale=np.linalg.norm([dx,dy]), label='quiver')
plt.legend(loc=2)
plt.show() |
Sorry, something went wrong.
|
Ok then, there is probably a need for vector to accept more than one set of data |
Sorry, something went wrong.
|
We've discussed this method in the dev call: https://hackmd.io/jd_7FjxNQ4y7XgNknvmvGQ#vector-API Basic ideas:
Summary:
Long story short, the color topic needs more investigation, we haven't reached a decision yet. |
Sorry, something went wrong.
|
As a user, I would be very frustrated if plt.vector did not follow the color cycle. I feel like this is a sensible default and can always be overridden through the "color", "ec", and/or "fc" kwargs. I guess I am missing the counterargument. |
Sorry, something went wrong.
I second @kmdalton that it should follow a color cycle. I find methods that don't to be extremely frustrating.
Probably not best discussed here but I've always wished that there was one global color cycle. It's disconcerting that scatter, plot and hist each have their own color cycle. That aside, I would advocate for the plot color cycle. |
Sorry, something went wrong.
|
If we are only making one arrow at a time, then I'm not convinced adding a new method is worth it just for the autolim and colorcycle behaviour. The colorcycle is dubious; I don't understand why we would want single arrows cycling their colors by default (groups of many arrows, sure). Similarly if I'm just adding one arrow at a time I typically have other data that I have plotted and that data set the limits for me. |
Sorry, something went wrong.
Here's a plot I made last week using plt.arrow to demonstrate complex addition for my students. I had to manually specify colors, limits, and otherwise override the broken default kwargs in plt.arrow. The whole process was frustrating and time-consuming. As you've noted, @jklymak , plt.arrow doesn't use FancyArrowPatch so the result isn't very aesthetically appealing either. import matplotlib.pyplot as plt
import numpy as np
F1, F2 = np.random.random(2) + 1j*np.random.random(2)
fig, ax = plt.subplots()
dx,dy,x,y= np.real(F1),np.imag(F1),0., 0.
ax.vector(dx, dy, x, y, label='F1')
dx,dy,x,y= np.real(F2),np.imag(F2),np.real(F1),np.imag(F1)
ax.vector(dx, dy, x, y, label='F2')
dx,dy,x,y= np.real(F1+F2),np.imag(F1+F2),0., 0.
ax.vector(dx, dy, x, y, label='F1+F2')
plt.xlabel(r"$\mathbb{Re}$", size=16)
plt.ylabel(r"$\mathbb{Im}$", size=16)
plt.grid(ls='-.')
plt.legend()
plt.show() |
Sorry, something went wrong.
|
The relevant comparison is not plt.arrow which we all agree is broken, but plt.annotate. I find your diagramming use case very specialized that you could easily write a little annotate wrapper for. I don't feel it justifies a whole new api element. But I'm just one vote. If others on the dev team think this is a good way to go, I'm not going to block it (unless it lacks unit support) |
Sorry, something went wrong.
|
I'm still not clear what the right way forward is. Let's maybe take one step back from what vector should do
to what is needed:
DiscussionRE 3: is out of scope here. Likely this wants a collection-type Artist, which we currently don't have for arrows (it was mentioned in the dev call that PatchCollection does not work with FancyArrowPatch, and that's not easily fixable). So let's save that for later. RE 2: "To create an arrow, create an annotation without text." is too convoluted. Semantically, a data arrow and an annotation are different things (and they may have different behaviors, e.g. concerning color-cycling). We need a dedicated function. Whether that'd be vector or fancy_arrow. RE 1: I'm pulling back a bit on the need to follow the arrow() API, as long as everything is still possible and it's reasonably simple to switch to the new API. |
Sorry, something went wrong.
|
We've discussed this extensively again in the dev call today: https://hackmd.io/jd_7FjxNQ4y7XgNknvmvGQ#Arrow Conculsions:
|
Sorry, something went wrong.
|
Sorry to miss the discussion - however, what was the argument for vector, other than to replace arrow? For multiple arrows we have quiver; for single arrows we have annotate. Our API problems are usually caused by too many similar ways to achieve the same thing (see pcolor,pcolorfast, pcolormesh), so I still don't understand why we want to add something that is basically just annotate with no option for accompanying text. |
Sorry, something went wrong.
As discussed above "To create an arrow, create an annotation without text." is awkward and not intuitive/discoverable. Moreover, they are conceptually different. Annotations are decorations, while we came to the conclusion that vector arrows should be considered as data. This implies that vector arrows take part in color cycling and can be added to the legend. |
Sorry, something went wrong.
|
This will be the only place in the library where "data" is only allowed to be represented by one set of values per method call. |
Sorry, something went wrong.
|
We don't have a suitable artist for multiple FancyArrowPatch (it doesn't work with PatchCollection because it needs special draw-time logic: an positioned in data space but it's shape must be drawn in screen space. 🤷 There's always the option to extend vector() later, but let's start small for now. |
Sorry, something went wrong.
|
Hi folks- just coming around to see if there's a way forward for this. Thanks! |
Sorry, something went wrong.
|
I think that there are good arguments for all the behaviors discussed in this thread. So as I see it, the next step needs to be matplotlib as a project (i.e. core devs) come to a concrete conclusion as to what the behavior/API should be. I'm happy to take over this PR and implement whatever that decision is, but cannot make it myself. Reading back more carefully I see that it has been very hard to reach true consensus. And also to define what counts as consensus (as not everyone can make the same dev meetings). Not entirely sure how to resolve that, but I standby that I will implement whatever once there is a definitive decision. My one request is that whatever the decision to e.g. default to black arrows, that there be a best effort made to preserve an easy way to opt-in to the other behavior such as color cycling/legend. Otherwise for our use case will just have to continue to use mpl-arrow |
Sorry, something went wrong.
|
I asked AI to summarize the discussion. I hope it's not too spammy. It's not perfect, but I think this gives a good starting point into the discussion after we all lost track of the topic. Edit / note: The summary is actually not that great. I believe it touches most of the relevant topics, but it's a bit unstructured, not precise and at times opinionated. prompt
Below is a structured summary of the API discussion in the GitHub thread you referenced (ignoring the PR implementation and focusing only on the discussion). The discussion centers on designing a new vector API in Matplotlib intended as a replacement for plt.arrow(). Detailed LLM summary 1. API Options That Were Considered1.1 Parameter ordering and representation of a vectorOption A — Same API as arrowvector(x, y, dx, dy, **kwargs)Meaning:
Motivation:
This option was explicitly recommended by maintainers as the preferred API. Option B — Direction-first vectorvector(dx, dy, x=0, y=0, **kwargs)Meaning:
Motivation:
Criticism:
Option C — Two-point representationvector(x0, y0, x1, y1, **kwargs)Meaning:
Motivation:
Drawback:
2. Number of Vectors per CallOption A — Single arrow onlyvector(x, y, dx, dy)Arguments:
Concerns raised:
Maintainer conclusion:
Option B — Allow arraysvector(x, y, dx, dy) # each could be arraysMotivation:
Counterarguments:
Thus arrays were considered but largely discouraged. 3. Relationship to Existing APIsSeveral alternatives were discussed: Using quiverPros:
Cons:
Conclusion: not a good replacement for a simple arrow/vector function. Using annotatePros:
Cons:
Thus the motivation for a dedicated function remained. 4. Keyword HandlingProposed behavior: vector(..., **kwargs) All kwargs passed directly to: FancyArrowPatch Reasons:
5. Color Behavior OptionsA major unresolved topic. Option A — Follow the color cyclePros:
Some developers strongly supported this. Option B — Do not follow the color cycleArguments:
Option C — Dedicated rcParamExample idea: rcParams["vector.color"] But this adds configuration complexity. Outcome: no decision reached. 6. Implementation BasisGeneral agreement:
Benefits:
7. Additional Concerns RaisedUnits supportx, dx, etc. must work with:
Example mentioned: similar to how errorbar handles deltas. Axis limits behaviorDesired behavior:
8. Consensus vs Open QuestionsAreas with Clear Consensus1. A replacement for arrow() is neededarrow() has known problems (e.g., axis-scale distortion). ([GitHub]2) 2. Use FancyArrowPatch internally3. Prefer API compatible with arrowFinal preferred signature: vector(x, y, dx, dy, **kwargs)4. Pass styling kwargs to FancyArrowPatch5. Purpose of vectorA simple way to draw a single arrow/vector without the complexity of annotate. Still Under Discussion1. Color-cycle behaviorQuestions:
2. Whether the function should support arraysOptions debated:
3. Whether the new API is necessary at allSome developers argued:
4. Exact argument order defaultsEspecially around:
One-Sentence SummaryThe discussion largely converges on vector(x, y, dx, dy) as a simple FancyArrowPatch wrapper replacing arrow(), but debates remain about color cycling, multi-arrow support, and whether the API is justified vs existing tools like annotate or quiver. |
Sorry, something went wrong.
PremiseI think we basically agreed on this, so I take this for granted:
Open topicsFrom my point of view, these are the open topics:
My opinion on the open topics
|
Sorry, something went wrong.
|
If we are making it be a single arrow, I'd agree with vector-ish(start, stop) in analogy to annotate(..., xy, xytext, ...). I think since this is obviously a subset of annotate's functionality, it should follow the annotate public API as closely as possible for everything, so that ideally annotate would just use vector-ish. As for the name, I'd actually do annotate_arrow so that it is clear it is the same thing as annotate, without the text. I'm not a fan of vector as that has a lot of connotations beyond just drawing an arrow. I strongly feel this should not be part of the color cycle. |
Sorry, something went wrong.
I think that's the whole problem though, far as I can tell what @ianhi and @kmdalton are after here is explicitly not annotate w/o text, which I think @timhoffm basically proposed at #29826 Also a lot of the power of the annotation arrow is the transforms, which is handled by ConnectionPatch.
I think that's fundamentally the sticking point, is this function supposed to have vector(data) or connect (annotate) semantics?
Maybe the API difference of connect(src, dest) and vector(x, y, dx, dy) is part of the underlying semantic difference/difference in use cases between the two functions?
Or we write a better tutorial for quiver (or maybe a wrapper over quiver?)
Which is why I think vector should if it's the replacement for the arrow that was a data method. ETA: B/c I think if you want an arrow for decoration, that's annotate w/o text. |
Sorry, something went wrong.
|
I think the main underlying difficulty is the "data" vs. "annotation" topic. If we limit us to one, most of the properties have obvious choices. Both are legitimate use case. We need to decide: I think we should do either b. or c. |
Sorry, something went wrong.
I strongly think we should do b as they're two distinct use cases and the attempts to do c. is: |
Sorry, something went wrong.
|
b also seems the easiest to me. a few ideas for names of either of the potentially two new functions: data like annotation like
I think the other was teh API for a data arrow. two points vs dx/dy. is it worth considering 3 functions? that would be the easiest to resolve, but adds the emost API surface to maintain. |
Sorry, something went wrong.
I would go with tuples, not flat coordinates. Four values in a row are harder to interpret than two tuples. With that, you can reasonably overload the signature. https://matplotlib.org/stable/api/_as_gen/matplotlib.axes.Axes.axline.html#matplotlib-axes-axes-axline is a precedent in that you can specify either a second point or a slope. Here you could do def data_arrow(from, to=None, *, direction=None):
# allowing
data_arrow((1,0), (1, 1))
data_arrow((1,0), to=(1, 1))
data_arrow(from=(1,0), to=(1, 1))
data_arrow((1,0), direction=(0, 1))or def data_arrow(*, from=None, to=None, direction=None):
# allowing
data_arrow(from=(1,0), to=(1, 1))
data_arrow(from=(1,0), direction=(0, 1))
data_arrow(to=(1, 1), direction=(0, 1))Not saying that we have to do this, but I would prefer that over two data functions. The trade off between the two is that you can use positional parameters in the first case (may be more convenient than always needing to give the parameter names), whereas in the second case, you can freely choose two out of (from, to, direction). |
Sorry, something went wrong.
|
I’m still not clear whether we need the two variants (b) or whether one variant can also work (c). What differences would we make?
Anything else? |
Sorry, something went wrong.
I think participation in the color cycle has been a major sticking point, but practically the big one is probably that an annotation arrow should support the annotation coordinate system while that's a lot of overhead for the data arrow. ETA: basically, data arrows should always have all points in the same transform, defaulting to data. Annotation arrows benefit greatly from the freedom of specifying head and tail in different coordinate systems, especially the relative positioning logic. ETA2: b/c GitHub just brought up this comment, data should participate in legends while annotations shouldn't. |
Sorry, something went wrong.
|
Poking even more on the transforms being a huge part of the story, if IndirectTransform goes in:
I'm not even sure how dx/dy gets computed if the start and end have different transforms. ETA: used IndirectTransform b/c it's a proposed GSOC, but this problem arises just out of supporting annotation coordinates. |
Sorry, something went wrong.
|
So we are at the following differences between data and annotation arrows:
Notes:
|
Sorry, something went wrong.
Except for the color cycle, the "data" in the above is just a basic subset of the "annotation" column. Edit: I guess except for the trivial math in start/end versus start/dx.
That is currently the way that Annotation is written, but I don't think the coupling is very complicated. |
Sorry, something went wrong.
To me that's the entire utility of annotation arrow/what I use it for. Also strictly speaking, I think data should not support different transforms for start and end - I think it's a hard constraint that data is specified in the same units. ETA: kinda like how scatter and line can't do per point transforms. |
Sorry, something went wrong.
Decoupling is basically the first step of the proposed IndirectTransform GSoC, and the IndirectTransform solves the problem of patching in relative positioning into ConnectionPatch. This PR of course doesn't have to wait on GSOC, but also this architectural stuff is part of why I think data and annotation have fundementally different scope/technical solutions. |
Sorry, something went wrong.
One is a strict subset of the other, isn't it? I would guess 75% or so of uses of annotate are from data-data space? I'm not familiar with the GSoC plan or what you are calling IndirectTransform, but Annotation already allows both ends of the arrow to be different transforms, so the problem is solved, it just needs a bit of refactoring. |
Sorry, something went wrong.
|
Also I'm a little frustrated here b/c I don't follow how a new arrow function that doesn't follow the color cycle and follows annotation semantics isn't mostly just: def ann_arrow(src, dest, src_coords=None, dest_coords='data', arrowprops=None, annotation_clip=None):
annotation = self.ax.annotate("", dest, src, dest_coords, src_coords, arrowprops, annotation_clip)
return annotation.arrow_patchThe other annotate kwargs are forwarded to text, and unwinding arrowprops is kind of a mess b/c it's based on arrow type. And sure that's (maybe) solving a problem, but I don't think that's solving the drawing vectors problem that motivated this PR. |
Sorry, something went wrong.
venn diagram, as demonstrated by the differences in Tim's table. data and annotate differ on the parameters specific to their use cases, which is kind of similar to imshow and matshow being different parameterizations of AxesImage. Or ConnectionPatch being FancyArrowPatch + coordinate transforms
Maybe, but almost none of mine. I'm pretty sure I just used arrow when I made arrows in data-data space, and then quickly realized I was using the wrong function b/c what I really needed was the positioning flexibility of annotate.
Except I think we should strictly not support this in the data arrow case b/c the expectation in the data case is that everything is in the same transform.
One of the proposals for this years GSoC1 is to:
Footnotes |
Sorry, something went wrong.
|
A new annotate arrow function would indeed only remove the text part of annotate. Again to summarize my position: I do not understand why you would want to have a special data->data arrow where the only extra feature is that it participates in color cycling, and it is much more restricted otherwise. It should either
But everyone should do what they want here. I don't think it's the end of the world to have one-off data->data arrows where each data point is plotted via a separate call. I just think it's crufty when I can already do the same thing with annotate. |
Sorry, something went wrong.
That may well be The broader topic here is that this all runs in the wake of substituting plt.arrow(). IMHO it's a major realization that people use arrows either as data or as annoations and that both are valid use cases that must be considered. While annotate / Annotation can draw an annotation arrow, it is not the right semantic API, because it's text-focussed. That justifies looking also at an annotate_arrow API. On the data arrow API, I just note that the existing plt,arrow() is far from usable, not only because of its distortions, but also because the end point is the head base, not the tip and it does not take part in color cycling. example plotplt.arrow(0, 0, 1, 1, width=0.03)
plt.arrow(1, 1, 1, -1, width=0.03)
plt.arrow(2, 0, -2, 0, width=0.03)
plt.grid()
So both parties have desires on an improved API. I would like to discuss both in proximity, because I still see a chance that one function can suffice. Please do not prematurely argue this cannot work because of ... Let's put the desired properties side by side, as I've started above. On a side note, I have been thinking about a more formal separation between data and decoration artists, but I'm now of the opinion that it's a spectrum. There are clearly data-only and decoration only cases, but there's also middle ground. This has to do with defining "data" is difficult. Is it that is purely (or partially) in data coordinates, is it that it can appear in a legend. Also the same Artist, e.g. a Line2D, can be used for data or decoration. So thinking about data or decoration use cases is good, but it's not necessarily helpful to encode this in the API. |
Sorry, something went wrong.
We have discussed this before. It's the wrong semantic model: "To draw an arrow, do a text annotation that uses an arrow style and leave out the text." |
Sorry, something went wrong.
I define it based on the semantic assumptions: The data artist $Artist: data-> graphic$ must preserve the structure of the data in the graphic s.t roughly $Artist( change (data)) -> change(graphic)$ The data arrow plotting function has to preserve the structure of the vector when scaled or translated - which is why I think the transforms are so key - for it to be a valid visualization. It's more of a nice to have for annotation - users will be frustrated if it doesn't, sure, but we've so far not guaranteed that data can be encoded in annotation the way it's encoded in say scatter.
That's why I support @ianhi and @kmdalton writing a data oriented replacement. I think the annotation side is just less urgent.
I think the differences are the sane defaults for each API, kinda like how we got to matshow/imshow:
|
Sorry, something went wrong.
|
Another way to think about this might be that the annotate "arrow" isn't really semantically an arrow. It's a line connecting the annotation to the thing it's annotating & that's why the start and end points are completely independent with respect to positioning/coordinate. The data arrow semantically is an arrow/vector - it's encoding a piece of data (often traditionaly represented as an arrow), which is akin to how a scatter mark encodes a point. And, like scatter, why the x/y axis values of the arrow head and tail are supposed to be semantically meaningful. Where I think this discussion is getting stuck now is in what I see as a proposal to use the data arrow as the annotation itself. If Annotation didn't provide an arrow, you'd (in theory) use AnnotationBbox to annotate the point with the arrow artist. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
This Draft PR is a first pass at implementing a vector method (see #22390) for the Axes class. Ideally someone can build on this to make a viable plt.arrow successor.
It uses FancyArrowPatch to draw the arrow. Right now the ArrowStyle is limited to "simple". It would be trivial to support "fancy". Other ArrowStyles will be more challenging as they diverge in parameter names.
This version supports legend labels out of the box through the Axes patch handler, and it automatically adjusts the axis limits. The arrow sizing can be adjusted in points. The scaling logic is based on the FancyArrowPatch code in plt.Annotate here.
Much of the credit goes to @ianhi for working out the details.
PR Checklist
Tests and Styling
Documentation