-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Setting logarithmic yscale in Radar chart broken #7751
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
Comments
I played a bit with
and indeed, with commit f8b1a9a the test script produces: while it seems quite OK with the previous commit (684861b) |
Attn: @anntzer |
Thanks for the bisect, that really simplies the process. |
I apologize, I forgot to mention one more thing in my previous post. Something may still be wrong, even with the commit 684861b , but I am not convinced it is the same issue : if some of the data values are “too small”, some weird artefacts appear in log scale. For example, this is the image produced when |
Something I do not understand between 684861b and f8b1a9a as it does not really seem related to the latter commit changes is the fact that |
Here's a script that demonstrates a difference introduced in commit f8b1a9a:
Before the commit I get
and after the commit
The old version multiplies the input data by the base before taking the logarithm, effectively adding one to the result. |
The following change seems to fix this test case, didn't test anything else: diff --git a/lib/matplotlib/scale.py b/lib/matplotlib/scale.py
index 17bc4e8..3bdc78f 100644
--- a/lib/matplotlib/scale.py
+++ b/lib/matplotlib/scale.py
@@ -99,7 +99,7 @@ class LogTransformBase(Transform):
def transform_non_affine(self, a):
with np.errstate(invalid="ignore"):
- a = np.where(a <= 0, self._fill_value, a)
+ a = np.where(a <= 0, self._fill_value, a * self.base)
return np.divide(np.log(a, out=a), np.log(self.base), out=a)
@@ -110,7 +110,7 @@ class InvertedLogTransformBase(Transform):
has_inverse = True
def transform_non_affine(self, a):
- return ma.power(self.base, a)
+ return ma.power(self.base, a-1)
class Log10Transform(LogTransformBase): |
Also, this is probably unrelated but in the same file LogitTransform replaces values >=1 with 1-1e-300, which is just the same as 1.0 since you don't get 300 decimals of precision near 1.0. |
This +/-1 was noted in #7144 with a "Why do we do this?" and subsequent removal. Given that this does not preserve backward compatibility and this has been around for 10 years (according to that PR), we should definitely revert that part of the change. |
Even as of 1.5.3 the MWE provided by @afvincent fails if one multiplies the data by 0.1 (and adding or removing the +/-1 doesn't change anything to it). Thus, I believe the change applied in #7144 simply modified the range of values for which the radar chart works correctly, but there is a deeper issue that needs to be fixed. The 1e-300 is related to #7413. Basically |
I think this script shows the root of the issue:
Any values <= 1 drop off a polar plot when it's |
And thinking about this a bit more, I think the problem is that after the log transform, some of the transformed values are negative, which a polar plot simply doesn't plot at the moment. Perhaps see #2133, also it looks like negative radial values work with a |
But it worked before. I have created many polar charts with log transformed values < 1. |
(If I am correct, a slightly horrible hack would be to replace the +/-1 that #7144 removed by +/-log([minimum-positive-float128]) (in the relevant base), which would handle all possibly relevant values :-)) |
I am very hazy on how the scale/projection system works and in particular how log scale interacts with the polar plots (and which unit systems all of these things are moving between). If these numbers never escape our internals, shifting to be strictly positive may be a reasonable path forward. |
@anntzer I'm pretty sure small values aren't showing up even in 1.5.3. You can see that if you run the example in the OP with a log scale. |
Actually it seems that very large values also get cropped out (replace 10 by 100 in your example), and adding an offset only shifts the working window instead of fixing the issue, so that's not going to be as simple. I don't mind having the +/-1 restored (with a pointer to this thread) so that the working range stays the same in 2.0 as in 1.5 :-) but the problem is obviously deeper... |
@tacaswell, @efiring, and myself discussed this and our consensus is:
|
Still an issue, I would say. Any updates? |
Still having issue here. |
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help! |
This example is now at https://matplotlib.org/stable/gallery/specialty_plots/radar_chart.html and the MWE example above fails in really weird ways (did not chase through why). Modifying the current example in a similar way (add minimum to data + setting logscale) works as does the log polar example above It does not look great, but I think that is because these data sets are not good for log data. I suspect that this was fixed by #24825 for mpl3.7 but have not bisected to test.
```python
import matplotlib.pyplot as plt
import numpy as np
from matplotlib.patches import Circle, RegularPolygon def radar_factory(num_vars, frame='circle'):
def example_data(): if name == 'main':
|
I have been using the API example for creating radar charts: http://matplotlib.org/examples/api/radar_chart.html
In the previous version you could make the radial grid logarithmic simply by adding
ax.set_yscale('log')
here:This seems to be broken in rc2, with no data being plotted.
The text was updated successfully, but these errors were encountered: