Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Recompute Wedge path after change of attributes. #1635

Merged
merged 1 commit into from Jan 16, 2013

Conversation

Projects
None yet
4 participants
Contributor

anntzer commented Jan 4, 2013

I changed some code for the Wedge patch to allow "draggable" wedges. Otherwise, the wedge's path was set in stone at initialization.

Owner

mdboom commented Jan 4, 2013

Maybe these new set functions should be implemented as properties so as not to change the interface?

Member

WeatherGod commented Jan 4, 2013

This partly harkens back to an issue I have been mulling over in that there needs to be a common interface across all artists to get/set the "data" of the artist. And it should probably be implemented as properties so that each subclass of artist can trigger any other appropriate recalculations as needed.

Contributor

anntzer commented Jan 8, 2013

Well, the common interface already exists in the getp and setp
functions. Sure, properties would be more pythonic, and I have little
love for getp and setp, but should we really add properties and thus
duplicate the "access points"? Of course I can do that if that's the
current "policy".

On Fri, Jan 04, 2013 at 06:19:52AM -0800, Benjamin Root wrote:

This partly harkens back to an issue I have been mulling over in that there needs to be a common interface across all artists to get/set the "data" of the artist. And it should probably be implemented as properties so that each subclass of artist can trigger any other appropriate recalculations as needed.


Reply to this email directly or view it on GitHub:
#1635 (comment)

Member

WeatherGod commented Jan 8, 2013

What I mean is a common property/attribute name across all artists. Some artists call it "data", while others call it "vertices", and others call it "xy". I should note that this qualm of mine shouldn't hold up this PR. It just that this PR highlights my qualm. There is no current policy.

Owner

mdboom commented Jan 16, 2013

@WeatherGod: do you agree this is good to go -- despite your qualm, which I share?

Member

WeatherGod commented Jan 16, 2013

It looks good to me. I see no reason not to take it. But it looks like it needs a rebase.

Owner

mdboom commented Jan 16, 2013

@anntzer : Would you mind rebasing your branch on the current master and repushing it to github?

Contributor

anntzer commented Jan 16, 2013

Here we go.

mdboom added a commit that referenced this pull request Jan 16, 2013

Merge pull request #1635 from anntzer/wedge-properties
Recompute Wedge path after change of attributes.

@mdboom mdboom merged commit e0325c8 into matplotlib:master Jan 16, 2013

Member

dmcdougall commented Jan 16, 2013

Cheers for your time and hard work @anntzer!

Thanks for ninjaing all these PRs, @mdboom! Teamwork!

@anntzer anntzer deleted the anntzer:wedge-properties branch Jan 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment