-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Return arrow collection as 2nd argument of streamplot. #803
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
I don't think this is a good idea; what is really needed is some sort of compound mappable artist. It doesn't have to be a new kind of collection, but it needs to be a single ScalarMappable so that when sci() is used and the colormap is changed, the lines and arrows change together. |
Hmm,... I was hoping it wouldn't come to that :) I guess I'll take a look at the ContourSet object (as you suggested in the original streamplot PR) and emulate it's behavior. BTW, after looking back at the original PR, I realized never flagged the streamplot function as experimental, as you suggested. I'm not really sure where that should be marked. Maybe in the return value itself (assuming that's the only major part that's likely change)? |
The ContourSet is a model in the sense that it produces and contains more than a single primitive Artist, and in that it requires extensive computation to generate those Artists, but it may not be an ideal model. It evolved a long time ago. It is possible that the Container base class would provide a good starting point. This is just speculation, though; I haven't looked closely. |
In the newest PR, |
The original purpose of the Container object was to support legend for complex plot commands. For example, errorbar command used to return a tuple of artists, thus I ended up with a simple Container class inherited from tuple. And it never meant to be a full implementation of artist. I'm not sure what the streamplot need to return. Maybe a single object of a class inherited from a ScallarMappable (and Artist). And I think it might not be a good idea to use the Container class for this purpose. |
Thanks for your comments, @leejjoon. After rereading @efiring's comments, I realize that he suggested that In my most recent commit, I change the base class to a simple object and give it attributes |
@tonysu, sorry for the extra work, but would you rebase this against master, please? Then I will merge it. I should have done so a long time ago. It definitely needs to be done before the RC. |
@tonysyu, also, while you are at it, you might want to note, in docstrings, and/or comments, something about the limitations and likely evolution. |
- Return object that derives from `object` since deriving from `Container` was not beneficial. - This return value is a stopgap; the final return value should allow users to set the colormap, alpha, etc. for both lines and arrows.
@efiring Rebased and added note about future changes to |
Hmm, Github seems to be screwing up the order of the commits. The discussion page for this PR shows commits in order: 1, 3, 2, 4. The commit page shows them in sequential order, and the diff page looks fine so it's probably just a bug in the discussion page. |
Return arrow collection as 2nd argument of streamplot.
Return arrow patches so users can modify or remove them after calling
streamplot
. This was listed as a todo in the original implementation, but never actually done.Maybe adding arrows to the axes should be delayed until the arrow collection is completely populated (and then the collection can be added to the axes all at once, just like the streamline collection)?