Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Use data limits plus a little padding by default#5583
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -898,6 +900,89 @@ def set_zorder(self, level): | |||
| self.pchanged() | |||
| self.stale = True | |||
| def get_top_margin(self): | |||
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.
can we do this via a property instead of getter/setters?
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.
Why be inconsistent with everything else? I know traits will eventually get us there, but we're not there yet...
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.
Also, does the auto kwarg handling handle properties?
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.
One less thing that we have to support back compatibility with.
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 is code in there to white-list properties to be updated.
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.
One less thing that we have to support back compatibility with.
I'm not really sold there -- I think it's more important to have everything working consistently than to have some things in the old way and some in the new better way... Do we already have other things that only exist as properties?
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 is inArtist.update.
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.
So there's only one property we currently whitelist:axes. I guess I'm willing to put this up to a "vote", but I'm strongly in favor of staying with setters/getters for now until we move over to traitletsen masse. There's just too much source for confusion otherwise.
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 have a local branch (that I was working on last night before I ended up down that formatter rabbit hole#5594) that uses a single code path forset,update, andsetp.
Thestale logic is implemented via a property and should (maybe) also be whitelisted so we are at two (both of which are my fault).
We should probably provide both, use the property version internally and advertise the getter/setter version until 2.1.
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.
👍 on the single code path for set/update/setp. I like that idea a lot.
I have a lot less objection doing both property and getter/setter. In fact, we can probably implement it fairly easily using the "old" pre-decorator property style. Yes, it will look archaic, but it's the best way to reduce code duplication IMHO.
def get_FOO(self): ...def set_FOO(self, val): ...FOO = property(get_FOO, set_FOO)Did you consider properties rather than getters/setters? |
I've added properties in addition to getters/setters here. |
I think this one is good to go. |
API: Use data limits plus a little padding by default
Lets see what the new examples look like |
Backported to 2.0.x as9617dfe |
API: Use data limits plus a little padding by default
Sorry I failed to backport this! |
This is part of the big style change overhaul for 2.0
Rather than using limits that are "round numbers", this will use the data limits plus a little padding.
The complication here is that not all plot types should have the padding. To solve this, it adds left/right/top/bottom margins for each artist which is a boolean. When autoscaling limits, the axes looks at all of its artists and any artist that explicitly sets a margin to
Falseon a given side means the whole axes will not have a margin on that side.Fixes#4891.