Skip to content
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

missing plt.close(fig) for heatmp.py? #1039

Open
xiaotiansu opened this issue Mar 13, 2025 · 1 comment
Open

missing plt.close(fig) for heatmp.py? #1039

xiaotiansu opened this issue Mar 13, 2025 · 1 comment
Labels
bug Something isn't working nonessential nice to have plotting

Comments

@xiaotiansu
Copy link
Contributor

Question

pm.plotting.heatmap(dataset.gaze[0]) gives me the same figure twice no matter show=True or show=False

Image

@dkrako
Copy link
Contributor

dkrako commented Mar 17, 2025

Good catch! For now, please just use plt.close() manually after plotting the heatmap.

Although we could just add the plt.close() to the function just like it's there with the rest, I wouldn't be in favor of this for the following reasons:

  1. As I remember the heatmap was originally implemented this way, to allow users to change the plot afterwards, for example by adding an additional background or else.
  2. Closing the figure could therefore break some workflows that rely on this. We will need to deprecate first.
  3. The workaround with manual plt.close()is easy and effective.

I'm not really sure how to handle this further. I would be definitely in favor of having consistent closing behaviour across all plotting methods.

Maybe this proposal would be the easiest to implement: add a close: bool | None = None argument to all plotting functions for the first iteration.

This way we can distinguish three cases:

  1. True: close fig
  2. False don't close fig
  3. None use the previously implemented behavior (i.e. False in heatmap, else True).

In a followup we would then need to decide which behavior we would like to use as a default in the future (close or not).
We would then raise a DeprecationWarning for the implementations that are inconsistent and if close is None.
This way we can safely deprecate old behavior and make it finally consistent subsequently a few versions later.

Example: if we want to close the figure by default, we would raise a DeprecationWarning in heatmap() if close is None with the message, that passing None is deprecated and True will be the default in v0.x.y.

This sounds more complicated than it is actually, but this would be the proper way on how to make this consistent without breaking user workflows.

@dkrako dkrako added plotting bug Something isn't working essential important nonessential nice to have and removed essential important labels Mar 17, 2025
@xiaotiansu xiaotiansu changed the title missing plt.close(fig) for heatmpa.py? missing plt.close(fig) for heatmp.py? Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working nonessential nice to have plotting
Projects
None yet
Development

No branches or pull requests

2 participants