Allow cmap argument override in ramanpsy.plot.image kwargs

[nihauc12]

Hello and first thx for this library, it looks really nice and we are considering building on it in our company :)

This PR is a small one: use case is if we want to do something like:

ax = ramanspy.plot.image(
    [data], cmap="inferno"
)

at the moment it raises an error:

TypeError: matplotlib.axes._axes.Axes.imshow() got multiple values for keyword argument 'cmap'

Since cmap argument is defined from color in the plot function.
This PR changes that so it takes cmap from plt_kwargs if present, if not it falls back to previous behaviour (building cmap from color)

Let me know what you think :)

[dgeorgiev21]

Thank you for your interest in contributing to RamanSPy. We are extremely happy to hear you find our framework useful!

We have reviewed your proposed changes and have the following comments:

• The code you are proposing can be simplified to something like:

if "cmap" not in plt_kwargs:
  white = [1, 1, 1, 0]
  plt_kwargs["cmap"]  = LinearSegmentedColormap.from_list('', [white, color])
        
im = ax.imshow(image, **plt_kwargs)

• It should also be noted in the documentation that the color argument will not be used if cmap is provided.
• Do you think similar behaviour would be useful for other methods - e.g. ramanspy.plot.volume?

---

If you find RamanSPy useful, please consider starring the project on GitHub! Your support means a lot!

[nihauc12]

this line was duplicated

[nihauc12]

Hello, thx for the review, good points, should have thought about the doc.

Could not use your simplification because the cmap variable was needed later in the call of the ax.figure.colorbar function.

So I changed the implementation a tiny bit.
The cmap argument is now declared as an argument of the function, defaults to None, in my opinion it makes the code a bit cleaner after.

Added the declaration in the docstring (should update the doc if I followed properly)

Added same modifications to the volume function.

Let me know what you think

[nihauc12]

Hello @dgeorgiev21 any news on this ?

[dgeorgiev21]

Hey @nihauc12 :)

We have not yet merged your changes because: 1) we wanted to finish setting up how we will be managing contributions and extensions (see our new contribution guide for more information); 2) we are still finalising the core infrastructure of the package.

We will start incorporating your changes and other extensions as soon as possible!

Thank you for your patience! :)

[dgeorgiev21]

Hi @nihauc12

First and foremost, we sincerely apologise for the delay in getting back to you. We truly appreciate your patience and the effort you’ve put into improving RamanSPy.

We have made a few slight modifications to your original idea to better align with the overall structure and goals of the project. Specifically, we have made the cmap argument the leading one, as we anticipate that this will be the preferred approach for the majority of users when utilizing these two plotting functions. The cmap argument will only be overridden if a color argument is explicitly provided. We believe this adjustment will enhance the overall usability and flexibility of these functions.

We have started integrating these changes into the cmap_override branch, which we are currently testing. Please feel free to review the changes we have implemented if you are interested. If everything looks good, we plan to merge this into the v0.2.11 branch, which will be included in our next release.

Again, thank you so much for your valuable contribution.