Asymmetrical diverging colormaps and vcenter

[gokceneraslan]

Hi,

I started playing around with vcenter and adding support for it in plotting functions. This allows us to do things like:

import scanpy as sc

adata = sc.datasets.paul15()

sc.pp.log1p(adata)
sc.pp.scale(adata)
genes = ['Cst3', 'Car1', 'Cd34', 'Apoe', 'Top2a', 'Ccl5', 'Ctsw']
sc.pl.matrixplot(adata, genes, groupby='paul15_clusters', vmin=-2, vmax=5, vcenter=0, cmap='RdBu_r')

I haven't gone further before asking you if it makes sense at all, or not. If yes, I can try to write tests too. What do you think?

[ivirshup]

I like this idea.

• While we should be conservative about adding new keywords, this fits well with vmin and vmax
• The docs for this argument should mention that users should pass a diverging palette with it, and probably have an example
• If norm is passed along at the same time, an error should be thrown
• It looks like there is a bunch of repeated code handling generating the norm, could this get put into a common utility function?

[ivirshup]

If norm is passed along at the same time, an error should be thrown

Following up on this a bit, I realized I didn't actually know what matplotlib would do if you passed norm and a bound, so I checked it out. Turns out they currently allow it, but it's deprecated, so throwing an error is the right thing to do.

import vega_datasets
import matplotlib as mpl, matplotlib.pyplot as plt

iris = vega_datasets.data.iris()

norm = mpl.colors.LogNorm()

plt.scatter(
    iris["sepalLength"],
    iris["sepalWidth"],
    c=iris["petalLength"],
    norm=norm,
    vmin=3,
)
plt.colorbar()

 MatplotlibDeprecationWarning: Passing parameters norm and vmin/vmax 
simultaneously is deprecated since 3.3 and will become an error two minor releases 
later. Please pass vmin/vmax directly to the norm when creating it.

[gokceneraslan]

 MatplotlibDeprecationWarning: Passing parameters norm and vmin/vmax 
simultaneously is deprecated since 3.3 and will become an error two minor releases 
later. Please pass vmin/vmax directly to the norm when creating it.

Yeah, that actually re-ignited my idea of adding support for vcenter after upgrading mpl :)

[gokceneraslan]

@fidelram Some help with the plotting tests is very much appreciated :) I don't get why only Python 3.6 test passes and others fail.

For example; why does this plot which is produced by this code where the color legend is defined here as Mean expression in group have the old title which is Expression level in group? If the "ground truth plots" in the "_images" folder are outdated, aren't the tests supposed to fail since the this commit i.e. Expression level in group -> Mean expression in group, but they don't fail, why?

Another example: I have updated this file but not that file which seem both outdated. But Python 3.6 test still seems to pass. How come? Are plotting tests disabled in Python3.6?

Plotting tests are extremely tricky and make it pretty difficult to contribute I must say :/ I think twice or three times when I want to change anything about plotting... (not a complaint addressed to @fidelram but something we should consider as a team)

[ivirshup]

Thanks for addressing my initial comments. I would definitely like @fidelram to take a look at it as well.

One addition I would like to see is a documented example with the expected plot and some prose. Maybe one for DE plots, one for embedding plots? This might be at home in the plotting tutorials.

I've requested some more changes in comments on the code

[fidelram]

@gokceneraslan Regarding the tests: yes, they are annoying particularly because is not possible to actually check why a test failed on the server while passes locally. I agree that this limits contribution because the mountain of work to get the tests working puts one off.

For the particular question about the title difference: the test may be passing because of the 'threshold' used to call the images as different. Why we use a threshold? This is to avoid tests from failing due to small differences between matplotlib or other graphic libraries versions or fonts installed. However, sometimes the threshold may be masking some small problems, although in general I am quite happy because important differences not missed.

BTW: The image that you point out is clearly wrong but I updated it recently for other reason (PR #1584).

Regarding the issue about adding norm as explicit parameter. I would suggest to add it if this just mean changing very few lines but I know this is lot of work (do we want tests for this?) for something that is already working.

Besides the very good review by Isaac I don't have much to add and will be happy to merge once some of the changes are taken care.

[codecov]

Codecov Report

Merging #1551 (cebeefc) into master (da66e15) will increase coverage by 0.08%.
The diff coverage is 95.45%.

@@            Coverage Diff             @@
##           master    #1551      +/-   ##
==========================================
+ Coverage   71.25%   71.33%   +0.08%     
==========================================
  Files          91       91              
  Lines       11090    11116      +26     
==========================================
+ Hits         7902     7930      +28     
+ Misses       3188     3186       -2     

Impacted Files	Coverage Δ
scanpy/plotting/_baseplot_class.py	84.46% <83.33%> (+0.65%)	⬆️
scanpy/plotting/_utils.py	54.79% <86.66%> (+0.87%)	⬆️
scanpy/plotting/_anndata.py	84.47% <100.00%> (+0.29%)	⬆️
scanpy/plotting/_docs.py	100.00% <100.00%> (ø)
scanpy/plotting/_dotplot.py	86.79% <100.00%> (ø)
scanpy/plotting/_matrixplot.py	97.87% <100.00%> (ø)
scanpy/plotting/_stacked_violin.py	83.67% <100.00%> (-0.09%)	⬇️
scanpy/plotting/_tools/__init__.py	76.27% <100.00%> (ø)
scanpy/plotting/_tools/scatterplots.py	86.95% <100.00%> (+0.15%)	⬆️