search

bittremieux / spectrum_utils

Python package for efficient mass spectrometry data processing and visualization
https://spectrum-utils.readthedocs.io/
Apache License 2.0
130 stars 21 forks source link

[Bug] Unpinned matplotlib version leads to unexpected results #42

Closed jspaezp closed 1 year ago

jspaezp commented 1 year ago

I noticed that plotting does not work in certain versions of matplotlib...

Here is the traceback...

File .../virtualenvs/.../lib/python3.9/site-packages/spectrum_utils/plot.py:178, in spectrum(spec, color_ions, annot_fmt, annot_kws, mirror_intensity, grid, ax)
    176 ax.yaxis.set_minor_locator(mticker.AutoMinorLocator())
    177 if grid in (True, "both", "major"):
--> 178     ax.grid(b=True, which="major", color="#9E9E9E", linewidth=0.2)
    179 if grid in (True, "both", "minor"):
    180     ax.grid(b=True, which="minor", color="#9E9E9E", linewidth=0.2)

That leads to this line: https://github.com/bittremieux/spectrum_utils/blob/437acb764c90d3b1bf14ded1d56d72c912e5fe41/spectrum_utils/plot.py#L178

And it breaks bebcause in matplotlib v3.5 that argument changed (to not have the b kwarg)

https://matplotlib.org/3.4.3/api/_as_gen/matplotlib.axes.Axes.grid.html

https://matplotlib.org/3.5.3/api/_as_gen/matplotlib.axes.Axes.grid.html

And since the version is not pinned, one can install either of them. https://github.com/bittremieux/spectrum_utils/blob/437acb764c90d3b1bf14ded1d56d72c912e5fe41/setup.cfg#L30

Solutions: Op1 -> Pin the version requirement of matplotlib to ">=3.4.0,<3.5.0" Op2 -> Pin a newer version and stop using the b argument.

LMK what you think Best! Sebastian

bittremieux commented 1 year ago

I think I've been using an older version of matplotlib the whole time and thus didn't run into this issue yet. The best solution is probably to change the code and require the newer version of matplotlib, as this will be more future-proof. Thanks for reporting!

jspaezp commented 1 year ago

Sorry for resurrecting this issue, do you believe this could be pushed as a hot fix to the release version in pypi? Thanks a lot! (LMK if there is anything you would like help with to make this happen)

bittremieux commented 1 year ago

Apologies, I can use a little kick under the butt sometimes. 🙂 I've included the fix in release 0.4.2, which is now live on PyPI.

jspaezp commented 1 year ago

wohooo! thanks a lot! (I get it, sometimes making time for OSS projects gets hard with life)