Skip to content

Better colormaps #82

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

Merged
merged 6 commits into from
Jan 17, 2014
Merged

Better colormaps #82

merged 6 commits into from
Jan 17, 2014

Conversation

mwaskom
Copy link
Member

@mwaskom mwaskom commented Jan 17, 2014

As you may know, I have strong opinions about colormaps.

This improves the colormap handling in PySurfer substantially. For add_data and add_contour_overlay, you can pass

  • A LUT array
  • The name of a matplotlib colormap (including the _r reversed colormaps)
  • A list of colors

and it will handle them appropriately.

I also changed the defaults so there are no evil rainbow colormaps in PySurfer.

@@ -10,6 +10,8 @@
import nibabel as nib
from scipy import sparse
from scipy.spatial.distance import cdist
import matplotlib as mpl
from matplotlib import cm
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should add a strong dependency on matplotlib. It wouldn't be used by people who supply their own LUT. I'd just nest the import in the functions, throwing an error if it doesn't import. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

We already imported matplotlib at the top level in surfer.viz

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, alright. I guess if it was already a dependency, it won't matter -- and most people have matplotlib anyway...

@larsoner
Copy link
Contributor

Other than my comment, LGTM, especially if it avoids using jet (did we use it before?)! Let me know once you've addressed my comment, and I should be able to run tests and merge tomorrow if you're done.

@@ -1,6 +1,7 @@
from os.path import join as pjoin

import numpy as np
import nose.tools as nt
Copy link
Contributor

Choose a reason for hiding this comment

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

why not from nose.tools import assert_equal to match our other uses?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just habit

@mwaskom
Copy link
Member Author

mwaskom commented Jan 17, 2014

We didn't have Jet, but blue-red was the default colormap for add_data (and the required map for add_contour_overlay), much to my shame. blue-red isn't as bad, as Jet, but there's no reason to ever use it.

@mwaskom
Copy link
Member Author

mwaskom commented Jan 17, 2014

Hold up on merging this, I need to update the contour activation example, as the new default isn't appropriate for a thresholded image.

@mwaskom
Copy link
Member Author

mwaskom commented Jan 17, 2014

OK should be good now. I forgot contour overlays are strictly positive.

larsoner added a commit that referenced this pull request Jan 17, 2014
@larsoner larsoner merged commit d295bfb into master Jan 17, 2014
@larsoner
Copy link
Contributor

Looks good, thanks!

@mwaskom mwaskom deleted the better_colormaps branch June 9, 2014 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants