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

Diverging cmap #698

Merged
merged 5 commits into from
Feb 29, 2024
Merged

Conversation

eugenioLR
Copy link
Contributor

@eugenioLR eugenioLR commented Feb 25, 2024

New arguments have been added called diverging and vmaxabs. They were added to the following functions: _SetupImage, imgplot, imghist, filterplot and ImageGrid.

The argument diverging forces vmin to be equal to -vmax, ensuring that diverging colormaps are displayed correctly, mapping the middle value to 0. The argument vmaxabs sets vmax to vmaxabs and vmin to -vmaxabs.

I added documentation and unit tests to everything that was needed.

This implements the features discussed in issue #697.

Copy link
Owner

@SarthakJariwala SarthakJariwala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together. I left a couple of comments that will have some minor changes throughout the PR but I didn't end up commenting everywhere.

Let me know if you have any questions.

src/seaborn_image/_filters.py Outdated Show resolved Hide resolved
src/seaborn_image/_core.py Outdated Show resolved Hide resolved
@SarthakJariwala SarthakJariwala added the enhancement New feature or request label Feb 27, 2024
@eugenioLR
Copy link
Contributor Author

I think that would solve the issues you raised. Thank you for your feedback.

Copy link
Owner

@SarthakJariwala SarthakJariwala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for working on this - looks good to me! I've left a few minor suggestions.

Lastly, can you add an example in the Examples section of the docstring in imgplot. It will help with feature discovery. If you would like to do it as a separate PR, I'm open to that too.

src/seaborn_image/_core.py Outdated Show resolved Hide resolved
src/seaborn_image/_core.py Outdated Show resolved Hide resolved
src/seaborn_image/_general.py Outdated Show resolved Hide resolved
src/seaborn_image/_general.py Outdated Show resolved Hide resolved
tests/test_general.py Outdated Show resolved Hide resolved
tests/test_general.py Outdated Show resolved Hide resolved
tests/test_general.py Outdated Show resolved Hide resolved
@eugenioLR
Copy link
Contributor Author

I adressed the changes you proposed, which seemed reasonable. The stuff with the comments was a mistake, it's now solved.

I also added the example in the docstring of imgplot, it didn't seem to be very hard at all, so it's not an issue. The only thing i worry about is if the example is meaningful enough, but it shows the feature so I think it's ok. Thanks again for you feedback.

@SarthakJariwala
Copy link
Owner

The example looks good for now. Thanks for working on this.

I'll release it as part of v0.9.0

@SarthakJariwala SarthakJariwala merged commit cab359c into SarthakJariwala:master Feb 29, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New feature for symmetic colormaps on images with negative values
2 participants