Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
polar plots do not properly handle units#4905
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
| angles = np.asarray(angles, np.float_) | ||
| self.set_xticks(angles * (np.pi / 180.0)) | ||
| else: | ||
| # The unit converters are defined to return radians |
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.
Silly question, where is this documented/implemented?
| self.set_xticks(angles * (np.pi / 180.0)) | ||
| if isinstance( angles[0], float ): | ||
| # If the data is floats it is assumed to be degrees (as per the | ||
| # docsting for this method) |
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.
docstring
Side note: there's no reason to prefix your commit messages with your initials; that's why you set your author info before committing anything. |
| @@ -399,6 +399,8 @@ def set_theta_offset(self, offset): | |||
| """ | |||
| Set the offset for the location of 0 in radians. | |||
| """ | |||
| # Make sure to strip away units | |||
| offset = self.convert_yunits(offset) | |||
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.
Andx here too.
Can you modify or extendhttps://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/test_axes.py#L384 to catch this issue? |
Added more test cases to test polar functions.
When moving my fixes over to git, I had accidentally transposed which axis was used for the unit conversion. I have fixed that. I have also added some more test cases to test for this. I am not sure what the process is for generating the baseline images or even running the test harness anymore. It has changed dramatically from when I first wrote it up so many years ago. What is the process for this now? |
See On Mon, Aug 17, 2015, 3:54 PM James Evansnotifications@github.com wrote:
|
Essentially, just run the test directly (after installing the latest On Mon, Aug 17, 2015 at 3:54 PM, James Evansnotifications@github.com
|
ping@jrevans Any update on this? |
| theta /= math.pi | ||
| # \u03b8: lower-case theta | ||
| # \u03c0: lower-case pi | ||
| # \u00b0: degree symbol | ||
| return '\u03b8=%0.3f\u03c0 (%0.3f\u00b0), r=%0.3f' % (theta, theta * 180.0, r) | ||
| s = '\u03b8=%0.3f\u03c0 (%0.3f\u00b0), r=%0.3f' % (theta, theta * 180.0, r) | ||
| s = s.encode( 'utf-8' ) |
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.
This shouldn't be necessary and probably indicates a bug somewhere else. Returning unicode from this function is actually what wewant. Can you describe which backend/platform you were using when you can across this problem?
@jrevans: Any progress on addressing the questions above? We may have to punt on this for 1.5.0. |
| @@ -411,6 +411,107 @@ def test_polar_units(): | |||
| assert_true(isinstance(plt.gca().get_xaxis().get_major_formatter(), units.UnitDblFormatter)) | |||
| def test_polar_format_coord(): | |||
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.
This needs a@cleanup decorator so global state does not leak.
I'm not going to close this one because it looks like it was close, but is there any chance it will get some movement? Someone who loves units will need to lead the charge... |
Closing this as not likely to be resurrected, but feel free to request a re-open. |
PolarAxes was not properly handling unitized data
This addresses an issue in#4897.