-
Notifications
You must be signed in to change notification settings - Fork 153
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
Modifications to polar graphs #2267
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2267 +/- ##
==========================================
+ Coverage 88.04% 88.06% +0.02%
==========================================
Files 247 247
Lines 23196 23279 +83
==========================================
+ Hits 20422 20500 +78
- Misses 2774 2779 +5
Continue to review full report at Codecov.
|
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.
Looks good! Just a couple of small comments - also the codestyle
CI job is failing and that is real, so could you fix the errors? (tox -e codestyle
to fix locally)
glue/viewers/scatter/viewer.py
Outdated
elif self.using_polar(): | ||
self.state.x_axislabel = "" | ||
# elif self.using_polar(): | ||
# self.state.x_axislabel = "" |
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 this code now be removed?
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.
Yes, has been removed
glue/viewers/scatter/viewer.py
Outdated
elif self.using_polar(): | ||
self.state.y_axislabel = "" | ||
# elif self.using_polar(): | ||
# self.state.y_axislabel = "" |
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 this be removed?
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.
Yes, has been removed
Could you also rebase this to pick up the latest CI fixes? |
…nd last tick mark (radius), rather than in the standard axis label positions.
7da97fc
to
f503a4a
Compare
I've rebased these commits onto the current master and added a changelog entry. Looks like all of the CI is passing except codecov/project. I'm not sure what the best way to handle that is. |
@Carifio24 - I think we can ignore the coverage failure, most changes here are covered (as shown by the codecov/patch job) |
This PR implements the changes to polar scatterplot styling discussed at the glue meeting on 1/13/2022. In particular, these changes are:
Rather than display axis labels along the bottom and side of the plot (which doesn't really make sense for a polar plot), the axis labels are included in the angular and radial tick marks. In particular, the angular tick at zero angle is of the form
label=0
, while the outermost radial tick has the formlabel=<value>
. Additionally, the radial tick marks are italicized to make the two sets of ticks clearly distinct.If the label is an empty string, the equals is omitted:
When in polar mode, the axis widget is adjusted accordingly: