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 log scale: fix inner patch boundary and spine location#30223
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
base:main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
| @@ -265,11 +265,17 @@ def _adjust_location(self): | |||
| self._path=mpath.Path.arc(np.rad2deg(low),np.rad2deg(high)) | |||
| ifself.spine_type=='bottom': | |||
| rmin,rmax=self.axes.viewLim.intervaly | |||
| ifself.axisisNone: | |||
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.
Added this check in case someone somewhere is using an arc spine that has not registered an axis. I'm not sure how likely that is though, and I did not find anything withthis search.
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'd say better safe than sorry, but is it possible to create a test case which triggers this? (Not saying that there should be, but maybe no harm to add one, especially if it doesn't require an image test.)
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 added a slightly contrived smoke test.
| @@ -265,11 +265,17 @@ def _adjust_location(self): | |||
| self._path=mpath.Path.arc(np.rad2deg(low),np.rad2deg(high)) | |||
| ifself.spine_type=='bottom': | |||
| rmin,rmax=self.axes.viewLim.intervaly | |||
| ifself.axisisNone: | |||
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'd say better safe than sorry, but is it possible to create a test case which triggers this? (Not saying that there should be, but maybe no harm to add one, especially if it doesn't require an image test.)
PR summary
Fixes#30179. The inner patch and spine positions are calculated in linear space, so we need to apply the scale's transform to min, max and origin.
The code from the OP now gives
The code from#30179 (comment) now gives
The test image looks like
I note there is an existing PR at#30185, but it appears to me unlikely to progress.
PR checklist