scanpy icon indicating copy to clipboard operation
scanpy copied to clipboard

DotPlot `.style()` resets previous value assignments

Open fidelram opened this issue 3 years ago • 1 comments

Multiple calls to sc.pl.Dotplot.style() cause resetting of parameters. This issue can be observed as a change in the default colormap shown by #1632 Other methods that set default parameters are also affected like .add_totals().

The following example should show the dots using the Reds colormap, but instead it uses the winter colormap because the second call sets the color map to winter if not given. This double call happens because when sc.pl.dotplot() is used (instead of sc.pl.DotPlot), internally a call to .style() is made and a subsequent explicit calls to .style() is required to tune the parameters as suggested in the documentation.

adata = sc.datasets.pbmc68k_reduced()
markers = ['C1QA', 'PSAP', 'CD79A', 'CD79B', 'CST3', 'LYZ']
sc.pl.DotPlot(adata, markers, groupby='bulk_labels').style(cmap='Reds').style(dot_edge_color='black').show()

image

The problem is caused by the current implementation of sc.pl.Dotplot.style() that set the default parameters as:

  def style(
        self,
        cmap: str = DEFAULT_COLORMAP,
        color_on: Optional[Literal['dot', 'square']] = DEFAULT_COLOR_ON,
        dot_max: Optional[float] = DEFAULT_DOT_MAX,
        dot_min: Optional[float] = DEFAULT_DOT_MIN,
        .....

Where DEFAULT_* are the default values defined at the beginning of the file (see https://github.com/theislab/scanpy/blob/master/scanpy/plotting/_dotplot.py#L84)

What is nice about this is that the documentation clearly shows the default values.

The downside is that optional values are assigned a default value that rewrites previous calls to style. Ideally, all optional values should be None, then is easy to know if a new value is passed or a previous call has already set a value. But, doing so will remove the defaults from the documentation.

@flying-sheep suggested to use a code he wrote to add default annotations to the documentation

https://github.com/theislab/scanpydoc/blob/875b441212830678cf9fc81c52f5af29bbb8715f/scanpydoc/elegant_typehints/formatting.py#L101-L107

fidelram avatar Feb 09 '21 12:02 fidelram

I think it would be fine for the parameter description to define the behavior. I don't think it particularly matters whether the default parameter is None or not. I think this is a pretty common solution to this issue.

ivirshup avatar Feb 10 '21 02:02 ivirshup