Add smart axis label visibility to wcsaxes #6774
Conversation
|
Hi there @Cadair 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here. |
| @@ -44,12 +44,13 @@ def set_visibility_rule(self, value): | |||
| if value not in allowed: | |||
| raise ValueError("Axis label visiblility rule must be one of{}".format(' / '.join(allowed))) | |||
|
|
|||
| self._visibility_rule = value | |||
| self._visiblility_rule = value | |||
There was a problem hiding this comment.
spelling mistake. should be self._visibility_rule = value
| @@ -19,6 +19,7 @@ def __init__(self, frame, minpad=1, *args, **kwargs): | |||
| self.set_ha('center') | |||
| self.set_va('center') | |||
| self._minpad = minpad | |||
| self._visiblility_rule = 'labels' | |||
There was a problem hiding this comment.
spelling mistake - should be self._visibility_rule
| self._visiblility_rule = value | ||
|
|
||
| def get_visibility_rule(self): | ||
| return self._visiblility_rule |
There was a problem hiding this comment.
Spelling mistake - should be self._visiblility_rule
| if value not in allowed: | ||
| raise ValueError("Axis label visiblility rule must be one of{}".format(' / '.join(allowed))) | ||
|
|
||
| self._visiblility_rule = value |
There was a problem hiding this comment.
spelling mistake - should be self._visibility_rule
| def set_visibility_rule(self, value): | ||
| allowed = ['always', 'labels', 'ticks'] | ||
| if value not in allowed: | ||
| raise ValueError("Axis label visiblility rule must be one of{}".format(' / '.join(allowed))) |
There was a problem hiding this comment.
spelling mistake - should be "visibility"
|
@astrofrog curious the docs fail is a real bug: https://travis-ci.org/astropy/astropy/jobs/290617410#L3451 Which interestingly is not covered by the tests. |
|
It passed! @astrofrog can you review this when you get a chance? |
CHANGES.rst
Outdated
| @@ -195,6 +195,11 @@ astropy.utils | |||
| astropy.visualization | |||
| ^^^^^^^^^^^^^^^^^^^^^ | |||
|
|
|||
| - Added automatic hiding of axes labels when no tick labels are drawn on that | |||
| axis. This parameter can be configured with | |||
| ``WCSAxes.coords[].set_visibility_rule`` so that labels are auto hidden when no | |||
There was a problem hiding this comment.
@Cadair: this should be set_axislabel_visibility_rule
This adds logic, and user preference, to determine under what conditions axis labels are drawn. By default if no tick labels are present the axis label will not be drawn.
The label position should be calculated based on all the bboxes on that axis not just the one for the current coord. This also attempts to fix the axislabels_regression test, but for some reason the label is still in a different place to before, even though the current behavior is correct.
Use all the bboxes for all the axes to calculate label position.
astrofrog
added
Ready-for-final-review
💤 merge-when-ci-passes
|
Thanks @Cadair! |
| @@ -467,6 +467,10 @@ astropy.visualization | |||
| ``WCSPixel2WorldTransform`` classes by setting ``has_inverse`` to ``True``. | |||
| In order to implement a unit test, also implement the equality comparison | |||
| operator for both classes. [#6531] | |||
| - Added automatic hiding of axes labels when no tick labels are drawn on that | |||
There was a problem hiding this comment.
FYI, I fixed this directly in master (2ece61c) since it's dead trivial and this got merged already
Add smart axis label visibility to wcsaxes
This adds configurable behaviour to wcsaxes so if there are no tick labels drawn on an axis the axis label will not be drawn. This can be configured so that the labels are always drawn or so that they are only drawn if ticks are drawn rather than tick labels.
@wafels @astrofrog