RamanSPy
RamanSPy copied to clipboard
Allow cmap argument override in ramanpsy.plot.image kwargs
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 :)
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
Hello @dgeorgiev21 any news on this ?
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! :)
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.