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

Axis Labels with offset Spines #4134

Merged
merged 9 commits into from Mar 12, 2015
Merged

Conversation

cimarronm
Copy link
Contributor

An attempt to fix the issue #3715. This takes into account the location of the axis spines when locating the axis label if there are no tick labels present.

Examples in #3715

Before patch

image

After patch

image

@cimarronm
Copy link
Contributor Author

Should probably add a test for this as well

@@ -1743,7 +1745,9 @@ def _update_label_position(self, bboxes, bboxes2):

else:
if not len(bboxes2):
top = self.axes.bbox.ymax
top = (self.axes.spines['top'].get_transform().
Copy link
Member

Choose a reason for hiding this comment

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

The docstring of the function only talks about tick labels, now the function uses spines too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Updated the docstrings

"""
if not self._autolabelpos:
return
x, y = self.label.get_position()
if self.label_position == 'bottom':
if not len(bboxes):
bottom = self.axes.bbox.ymin
Copy link
Member

Choose a reason for hiding this comment

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

Would it make more sense to just throw the bounding box of the spine and the axes in to the bboxes list and let the union call take care of this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, @tacaswell, I think that makes more sense. I'll update to do just that.

@tacaswell tacaswell added this to the next point release milestone Feb 20, 2015
… all ticklabels and axis spine for use in label placement
else:
bbox = mtransforms.Bbox.union(bboxes)
bottom = bbox.y0
spinebbox = (self.axes.spines['bottom'].get_transform().
Copy link
Member

Choose a reason for hiding this comment

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

Style alternative: I would factor out spine = self.axes.spines['bottom'] so as to make the spinebbox expression more compact.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that is much better. Will modify it as so

@efiring
Copy link
Member

efiring commented Mar 1, 2015

Looks good. A test would be nice.

@cimarronm
Copy link
Contributor Author

Working on a test

…no tick labels are present and axis spines are offset (see issue matplotlib#3715)
…ng pep8 now. Modified test_spines to not use assert_less since it is not available in python 2.6
tacaswell added a commit that referenced this pull request Mar 12, 2015
BUG : Axis Labels with offset Spines

Closes  #3715
@tacaswell tacaswell merged commit e4222e8 into matplotlib:master Mar 12, 2015
@tacaswell
Copy link
Member

Thank you

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

4 participants