Skip to content

Conversation

@larsoner
Copy link
Member

Some simple code to plot with hot_r with transparent=True and alpha=1. (should yield an opaque volume where values are large) looks like this on master:

Screenshot from 2020-12-15 12-38-12

And on this PR it's correct:

Screenshot from 2020-12-15 12-37-18

The key was the opacity and color transfer functions both need to yield floats between 0 and 1, so we just need to divide both by 255 (ctable is np.int8) rather than doing whatever silly gymnastics we were doing before where the colors ranged from 0 to 255 and the alphas 0 to 1/255 (effectively).

While I was messing around with this I noticed add_data could not use colormap='mne' so I refactored our get_cmap calls. I also added a simple test to test_brain to make sure our ctable is correct.

@GuillaumeFavelier can you take a quick look? Not sure if this or #8335 should go in first, but I think they're going to conflict.

@larsoner larsoner added this to the 0.22 milestone Dec 15, 2020
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM. And don't worry about the conflicts, I'll fix these in #8335 👍

@larsoner larsoner merged commit 88d9be9 into mne-tools:master Dec 15, 2020
@larsoner
Copy link
Member Author

Thanks for the quick review @GuillaumeFavelier !

@larsoner larsoner deleted the fix-vol branch December 15, 2020 18:40
larsoner added a commit to agramfort/mne-python that referenced this pull request Dec 16, 2020
* upstream/master: (42 commits)
  MRG, ENH: Add DICS bias tests (mne-tools#8610)
  MRG, BUG, ENH: Add window option (mne-tools#8662)
  BUG: Fix alpha for volumes (mne-tools#8663)
  MRG, BUG: Fix bugs with envcorr (mne-tools#8658)
  MRG, ENH: Progressbar for csd_morlet (mne-tools#8608)
  Render is necessary now (mne-tools#8657)
  VIZ: Fix head size (mne-tools#8651)
  MRG, MAINT: bump sphinxcontrib-bitex version (mne-tools#8653)
  MRG, MAINT: Improve server env (mne-tools#8656)
  BUG: Mayavi center (mne-tools#8644)
  VIZ, ENH: allow show/hide annotations by label (mne-tools#8624)
  Add regression test for EEGLAB data with a chanlocs struct (mne-tools#8647)
  FIX: scalar_bar (mne-tools#8643)
  MRG: Small fix to tutorial; rename plot_events ordinate label to "Event id"; improve some SSP docstrings (mne-tools#8612)
  MRG, ENH: make plot alignment use defaults for colors (mne-tools#8553)
  BUG: Fix passing of channel type (mne-tools#8638)
  FIX: fixed loop over norm PSF/CTF options (mne-tools#8636)
  MRG, BUG: Pass kwargs (mne-tools#8630)
  DOC: Clearer error message (mne-tools#8631)
  BUG: Fix number of labels (mne-tools#8629)
  ...
larsoner added a commit to wmvanvliet/mne-python that referenced this pull request Dec 16, 2020
* upstream/master: (38 commits)
  MRG, ENH: Add DICS bias tests (mne-tools#8610)
  MRG, BUG, ENH: Add window option (mne-tools#8662)
  BUG: Fix alpha for volumes (mne-tools#8663)
  MRG, BUG: Fix bugs with envcorr (mne-tools#8658)
  MRG, ENH: Progressbar for csd_morlet (mne-tools#8608)
  Render is necessary now (mne-tools#8657)
  VIZ: Fix head size (mne-tools#8651)
  MRG, MAINT: bump sphinxcontrib-bitex version (mne-tools#8653)
  MRG, MAINT: Improve server env (mne-tools#8656)
  BUG: Mayavi center (mne-tools#8644)
  VIZ, ENH: allow show/hide annotations by label (mne-tools#8624)
  Add regression test for EEGLAB data with a chanlocs struct (mne-tools#8647)
  FIX: scalar_bar (mne-tools#8643)
  MRG: Small fix to tutorial; rename plot_events ordinate label to "Event id"; improve some SSP docstrings (mne-tools#8612)
  MRG, ENH: make plot alignment use defaults for colors (mne-tools#8553)
  BUG: Fix passing of channel type (mne-tools#8638)
  FIX: fixed loop over norm PSF/CTF options (mne-tools#8636)
  MRG, BUG: Pass kwargs (mne-tools#8630)
  DOC: Clearer error message (mne-tools#8631)
  BUG: Fix number of labels (mne-tools#8629)
  ...
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