labsyspharm / scimap

Spatial Single-Cell Analysis Toolkit
https://scimap.xyz/
MIT License
74 stars 26 forks source link

overlapping legend on sm.pl.heatmap when constrained_layout set to True, and default arguments #108

Closed emmanuel-contreras closed 3 months ago

emmanuel-contreras commented 4 months ago

A couple potential issues here with sm.pl.heatmap 1) _constrainedlayout set to True causes the legend to overlap the heatmap (see images below), setting it to False fixes this issue but I don't think it can be done as an argument to the heatmap function. https://github.com/labsyspharm/scimap/blob/3e68c793ebe50bb28c087d45618b28bca2b4f92d/scimap/plotting/heatmap.py#L273

2) ValueError: Both 'saveDir' and 'fileName' must be provided together or not at all.. fileName argument has a default value of 'heatmap.pdf', given the error, it may make more sense to set it to None by default.

https://github.com/labsyspharm/scimap/blob/3e68c793ebe50bb28c087d45618b28bca2b4f92d/scimap/plotting/heatmap.py#L49

image

image

ajitjohnson commented 4 months ago

Thank you for reporting the bug @emmanuel-contreras. Why do think passing constrained_layout as a parameter would not work?

emmanuel-contreras commented 4 months ago

ohh, what I meant is that some refactoring will have to be done since constrained_layout is hard coded to True at the moment. But please correct me if I'm wrong.

ajitjohnson commented 4 months ago

Yes, I will need to investigate why the subpanels are not behaving as expected. The initial reason for using constrained_layout was to prevent the legends from moving outside the visible area.

As a quick fix, I could expose the parameter. Would that be helpful? Nonetheless, it is essential to determine the underlying cause.

emmanuel-contreras commented 4 months ago

Got it, yea that makes sense. I've been looking a bit more into this and it looks like the overlap is coming from the fixed positioning of the colorbar in these lines.

https://github.com/labsyspharm/scimap/blob/296885b512120aef3a622605159d741ad9ff03a8/scimap/plotting/heatmap.py#L303-L304

Looking at the colorbar positioning documentation I repositioned it with _insetaxes, and it seems to work while leaving constrained_layout set to True if you wanted to try it. Otherwise exposing constrained_layout would also work for now. https://matplotlib.org/stable/users/explain/axes/colorbar_placement.html#using-inset-axes

cbar_ax = ax.inset_axes([-.5, -1.5, 4, .5], transform=ax.transData)
cbar = fig.colorbar(c, cax=cbar_ax, orientation='horizontal')
ajitjohnson commented 3 months ago

hi @emmanuel-contreras sorry for the delay. I did not notice your message. I have released a new version with your changes.

emmanuel-contreras commented 3 months ago

No worries, thanks for merging the changes and for developing a great package!