-
Notifications
You must be signed in to change notification settings - Fork 19
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
ChartVisualizer not compatible with matplotlib 3.10 #270
Comments
pyRDDLGym.core.visualizer.chart.render() use a deprecated method which is even not exising anymore starting from matplotlib==3.10. And this is used by default when rendering the RDDLDomain wrapping pyRDDLGym environments. See issue pyrddlgym-project/pyRDDLGym#270
pyRDDLGym.core.visualizer.chart.render() use a deprecated method which is even not exising anymore starting from matplotlib==3.10. And this is used by default when rendering the RDDLDomain wrapping pyRDDLGym environments. See issue pyrddlgym-project/pyRDDLGym#270
I think I have fixed this issue, by switching to the buffering in the io package instead of the matplotlib one. I will need to do a little more testing before I merge though. You are welcome to check out the new PR and let me know if that fixes your problem if you have the time. Thanks. |
Thanks @nhuet for raising this issue. @mike-gimelfarb It seems like the simple one line fix is to use buffer_rgba(), but I gather your fix that saves the fig as a png to an in-memory buffer is intended to simplify the code and make it more immune to future matplotlib changes? Note: This issue seems to have broken a large number of our visualizers... same issue in Ping, Tetris, Reservoir, UAV, to name a few. For the time being, users may wish to downgrade to matplotlib==3.9.4 if they encounter this issue. |
My only concern with the one line fix is that I couldn’t find when that function was first provided. If it is available from old enough versions of matplotlib then I can implement the one line fix for everything. If it is not, then I will likely break installations on older Python versions, which will be annoying to fix. My understanding is that it has been around for a while because of the deprecation warning noted above, so maybe I shouldn’t worry about it and just make the one line change to all the visualizers. What do you think? |
Apparently there is also a problem with that function on certain backends as shown here so that is quite annoying. I think I actually encountered this when I was trying to make the change on windows, but I’ll have to double check. |
Ah, yes, this last point is an excellent reason to move to the more general fix in your pull request #217 (and in the other visualizers affected by this issue). |
I’m only asking since I’m still not entirely sure which fix we should implement. I will try to test again the recommended fix above again to see if it was an issue with packages on my side. The fix above would be (1) easier as it is just a one line change, and (2) it doesn’t rely on an extra package, even if it is built-in. |
ChartVisualizer.render()
calls internallyFigureCanvasAgg.tostring_rgb()
which has been removed in matplotlib 3.10. In previous versions it was deprecated and it was proposed to useFigureCanvasAgg.buffer_rgba()
instead, maybe this is the solution?This is a minimal example of code failing with matplotlib 3.10 and pyrddlgym 2.1:
The text was updated successfully, but these errors were encountered: