-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Allow the user to specify a figure to reuse when creating a FacetGrid #2830
Comments
How does this option work together with the |
This has definitely come up before, I think I have articulated some thoughts. Basically, making this possible is straightforward; your approach is the right one. The blocker has always been thinking through all the edge cases that would result from enabling this, given that @jhncls hits on several of the important points. I assume that the main usecase here is when working with a The main thing is the fact that The edge cases around the size parameters in the constructor would take some care — you'd want to fire a helpful warning if they are used, but they current have non-null default values. This adds some complexity, although I think the API decisions are reasonably straightforward. There may be other considerations I am not immediately thinking of. Another thing to note: this would need to be rolled out to the other |
I'll take a look at these cases and try to formulate a coherent plan. I think the general strategy is going to be: if the user is specifying the figure, they are responsible for setting it up in a "clean" way, and if they don't then results may not be expected (although I agree it's a good idea to fire off a warning if we can detect these cases). For all API decisions, the defaults when For
I'm in favor of the "auto" approach, and if they are specified as numbers they are always applied. I think the same approach will work with calling "tight_layout". The legend issue is a tricky one that I will have to think about, but I think it can be dealt with in a sensible way.
That is a use case. But I simply want to control the number of figures on my screen by assigning them all numbers. I don't want FacetGrid to always pop open a new figure. I want it to reuse an existing one. I don't think I care if it clears it. I might care if it is resized, but it probably isn't that big of a deal.
I'll probably work on this on and off. I think I care enough about it where I'm not going to abandon it, but I'm also more interested in just having the feature rather than working on implementing it. So if anyone wants to get to it before I do that would be helpful. |
Easy enough to say, but you're not going to be the one dealing with the bug reports ... |
You can always @ me 😉 More seriously, I do think it's the right approach and I think we can set this up in a way to minimize foot-shooting. |
To be clear — I am in favor of adding this functionality (and it already exists in the new objects interface). I just expect it to take some careful thought about how to handle all the edge cases. |
@Erotemic I'm curious, would it suit your use case if you could pass in something like Does that approach strike a good balance between allowing the user some control over the figure creation, without opening the door to likely problems? |
Yes, that would likely be sufficient. |
@Erotemic the below is a horribly hacky workaround, but it will allow you to open one figure window, and the re-execute your code and have it re-use that same figure window. It hijacks the call from Seaborn into if "figure_original" not in locals():
figure_original = plt.figure
def figure_new(**kwargs):
import inspect
if "seaborn" in inspect.stack()[1].filename and "num" not in kwargs:
print("Seaborn calling plt.figure()")
kwargs["num"] = "Seaborn Figure"
kwargs["clear"] = True
return figure_original(**kwargs)
plt.figure = figure_new |
I just started using Man... I just want to control the figure before I send it to seaborn. I really don't like that monkey patch. I want my tools to interoperate with others, and if I monkey patch pyplot something is going to break. But yeah, just passing I made a slightly longer version of the monkey patch that uses a context manager to clean up after itself that I'm going to try in the meantime: class MonkeyPatchPyPlotFigureContext:
"""
😢 🙈 😭
Forces all calls of plt.figure to return a specific figure in this context.
References:
..[Seaborn2830] https://github.com/mwaskom/seaborn/issues/2830
CommandLine:
TEST_MONKEY=1 xdoctest -m kwcoco.cli.coco_visual_stats MonkeyPatchPyPlotFigureContext
Example:
>>> # xdoctest: +REQUIRES(env:TEST_MONKEY)
>>> from kwcoco.cli.coco_visual_stats import * # NOQA
>>> import matplotlib.pyplot as plt
>>> func1 = plt.figure
>>> self = MonkeyPatchPyPlotFigureContext('mockfig')
>>> with self:
>>> func2 = plt.figure
>>> func3 = plt.figure
>>> print(f'func1={func1}')
>>> print(f'func2={func2}')
>>> print(f'func3={func3}')
>>> assert func1 is func3
>>> assert func1 is not func2
"""
def __init__(self, fig):
from matplotlib import pyplot as plt
self.fig = fig
self.plt = plt
self._monkey_attrname = '__monkey_for_seaborn_issue_2830__'
self._orig_figure = None
def figure(self, *args, **kwargs):
"""
Our hacked version of the figure function
"""
return self.fig
def _getmonkey(self):
"""
Check if there is a monkey attached to pyplot
"""
return getattr(self.plt, self._monkey_attrname, None)
def _setmonkey(self):
"""
We are the monkey now
"""
assert self._getmonkey() is None
assert self._orig_figure is None
# TODO: make thread safe?
setattr(self.plt, self._monkey_attrname, 'setting-monkey')
self._orig_figure = self.plt.figure
self.plt.figure = self.figure
setattr(self.plt, self._monkey_attrname, self)
def _delmonkey(self):
"""
Get outta here monkey
"""
assert self._getmonkey() is not None
assert self._orig_figure is not None
setattr(self.plt, self._monkey_attrname, 'removing-monkey')
self.plt.figure = self._orig_figure
setattr(self.plt, self._monkey_attrname, None)
def __enter__(self):
current_monkey = self._getmonkey()
if current_monkey is None:
self._setmonkey()
else:
raise NotImplementedError('no reentrancy for now')
def __exit__(self, ex_type, ex_value, ex_traceback):
if ex_traceback is not None:
return False
self._delmonkey() |
Currently
FacetGrid
and its consumersrelplot
andcatplot
will always create a new figure when you call them. In my workflow, I often like to create the figures or axes beforehand and then pass these as arguments to seaborn functions. This works with axes level plots linelineplot
andscatterplot
, where you can pass theax
parameter, and if you don't it usesplt.gca()
.I would like to propose that
FacetGrid
should take a parameterfig=None
, and if specified it uses it, otherwise it uses the existing code as such:That would allow me to force specific plots to be associated with specific figure numbers, which helps me keep track of things as I work. I can make this PR if there is interest in this feature. I made a draft here: #2831 and I will work on the remaining bullet points if there is interest.
The text was updated successfully, but these errors were encountered: