Skip to content
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

Rotate markers in Scatter plot #2432

Closed
wants to merge 27 commits into from
Closed

Rotate markers in Scatter plot #2432

wants to merge 27 commits into from

Conversation

mgoacolou
Copy link
Contributor

Make possible to give a scalar or an array for rotate markers. This is usefull for representing for example Speed, Type, Direction of mobile targets.

self.__check_parameters()

def __check_parameters(self):
if self._sizes is not None and self._angles is not None and self._sizes.size != self._angles.size:
Copy link
Member

Choose a reason for hiding this comment

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

Line too long.

@pelson
Copy link
Member

pelson commented Sep 18, 2013

@mgoacolou - I really like this new feature. I've put quite a number of comments on this PR - please don't be disheartened, I think what you've implemented has been done cleanly and is in a really good place, and I see no reason why this can't be merged in relatively short order.

You should add an entry in the what's new of the documentation (link to the new gallery entry too). I think the example could do with a bit of work and it can go under the new "lines_bars_and_markers" section of the gallery.

Great work! 😄

@pelson
Copy link
Member

pelson commented Sep 18, 2013

P.S. This needs rebasing before we can consider merging (it wont currently merge cleanly).

mgoacolou and others added 9 commits September 18, 2013 14:38
The docs now state that creation of a proxy artist is the preferred method for
creating legends on stackplots.  This is because stackplot creates
PolyCollection objects which are not supported by the legend
Mixing 'ha' or 'va' with AnchoredText produces bad output, so add a warning in
this case.
Use the `n` format specifier instead of `l` to build Python integer or long integer objects from size_t values.
@@ -3072,7 +3072,7 @@ def quiverkey(*args, **kw):
# This function was autogenerated by boilerplate.py. Do not edit as
# changes will be lost
@_autogen_docstring(Axes.scatter)
def scatter(x, y, s=20, c='b', marker='o', cmap=None, norm=None, vmin=None,
Copy link
Member

Choose a reason for hiding this comment

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

This is generated. I don't think you should be modifying it.

@pelson can you confirm?

Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing the generation has modified it. We can check that by re-running boilerplate though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not know he the script witch generate this file. So I run it and it generate it

@WeatherGod
Copy link
Member

We are going to need tests. I also wonder how this would impact some of the optimizations that have been done recently for scatter(). I also wonder if this feature should be brought into the plot() family of functions?

One thing I would certainly be interested in is if barbs() and quiver() could benefit from this?

@pelson
Copy link
Member

pelson commented Sep 18, 2013

One thing I would certainly be interested in is if barbs() and quiver() could benefit from this?

To represent quantities that aren't east and north (u/v)? Obviously that is out of scope for this PR, but an interesting prospect for future work.

@WeatherGod
Copy link
Member

I was more referring to the notion that now that PathCollection supports an angles argument, then maybe some other Collections that we have that are subclasses of PathCollection could benefit from passing off the angle handling down to PathCollection. But, now I see that nothing is a subclass of PathCollection, and several of the markers aren't PathCollections, either. So, I wonder if this would break certain markers? Particularly asterisk and star markers (they are PolyCollections).

@pelson
Copy link
Member

pelson commented Sep 19, 2013

Ah, have you merged master into this branch? Something has gone terribly wrong (look at the diffs). It is strongly encouraged that you rebase instead of merge master, but I'm surprised it has had the effect it has.

Would you mind having another look at why the commits are all messed up?

Thanks,

@tacaswell
Copy link
Member

@mgoacolou Closing this PR because you opened #2478.

@tacaswell tacaswell closed this Jan 5, 2014
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.

None yet

8 participants