Skip to content
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

Pin version of matplotlib #2863

Closed
h-mayorquin opened this issue May 16, 2024 · 7 comments · Fixed by #2866
Closed

Pin version of matplotlib #2863

h-mayorquin opened this issue May 16, 2024 · 7 comments · Fixed by #2866
Labels
dependencies Issue/PR that is related to dependencies packaging Related to packaging/style

Comments

@h-mayorquin
Copy link
Collaborator

Also from #2861.

We are getting the following error:

E       AttributeError: module 'matplotlib.cm' has no attribute 'get_cmap'

We need to figure out when this attribute was introduced or deprecated. The version used is 3.9 there https://github.com/SpikeInterface/spikeinterface/actions/runs/9112219162/job/25051018131?pr=2861 so most likely a deprecation.

@h-mayorquin h-mayorquin added dependencies Issue/PR that is related to dependencies packaging Related to packaging/style labels May 16, 2024
@h-mayorquin
Copy link
Collaborator Author

Ok, is the latest version that was just released:

https://matplotlib.org/stable/api/prev_api_changes/api_changes_3.9.0.html#top-level-cmap-registration-and-access-functions-in-mpl-cm

I am not familiar with plotting. I think it is up to @alejoe91 and @samuelgarcia if they want to add a upper bound or change the code to work with matplotlib latest. If they are too busy and this does not move on on the following days I will put a ceiling on the version so avoid failing tests and then we can revert once someone has time to update the code.

@JoeZiminski
Copy link
Collaborator

JoeZiminski commented May 16, 2024

Thanks @h-mayorquin! I would advocate to fix the error as you suggest rather than pin matplotlib. If we pin we'll have to deal with it another time anyway and may forget to fix it. If using matplotlib 3.9.0 is causing errors not currently caught by the test suite, these can be fixed + tests added to extend coverage.

@h-mayorquin
Copy link
Collaborator Author

Yes, it is more than this is a busy time for us.

I have never seen testing of matplotlib that I feel is satisfactory. Automatizing plot output testing seems like a difficult thing to do (see the testing suit of ipywidgets). Have you seen or done that in a way that feels good?

@JoeZiminski
Copy link
Collaborator

JoeZiminski commented May 16, 2024

Yes good point about it being a busy time, how about we fix this one, but if it is leading to a lot of errors we can pin until things are less busy?

Yes I agree testing plots is always unsatisfying, you can never be sure the plot is rendering visually as you'd expect. I think the best you can do is to check that the expected axes, axes limits, titles and data are shown on the plot across all possible input arguments. For example I added some tests recently for testing plot_motion that tests some of these things. At the end of the day you cannot be sure it is rendering as expected, but if the API is reporting it is then that is a problem on the matplotlib side I guess. It's kind of a similar problem to testing GUIs, the textual test suite has some nice examples of this.

@samuelgarcia
Copy link
Member

samuelgarcia commented May 17, 2024

Hi.
I will make the ptach for this color stuff super soon.
I have the same problem in any package I did

@h-mayorquin
Copy link
Collaborator Author

I will keep this open until Sam updates the issue just as a todo.

@h-mayorquin
Copy link
Collaborator Author

This was solved by #2891

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Issue/PR that is related to dependencies packaging Related to packaging/style
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants