Add external context mode for axes #406
Conversation
|
Note that ideally we just ask the seaborn developers to make their private labels available with |
|
Note to self -- run the suite before pushing PRs! |
|
The import statements are handled by my editor -- bit annoying. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Will add a few more tests to reach the target and then open it for review |
beckermr
left a comment
beckermr
left a comment
There was a problem hiding this comment.
The frame detection seems fragile, to say the least.
Can we use an alternate mechanism where a user can mark a subplot as external to ultraplot and ultraplot uses this mark internally to basically turn itself off?
This would be more general since it could be used with other plotting packages and it would be more robust since we would not rely on automated detection of arbitrary external plotting code.
|
You’re right that stack-frame detection is brittle. I’ve added an explicit, opt‑in “external mode” so users can mark an axes as being driven by an external plotting library. When external mode is on, we defer on-the-fly guide creation (legends/colorbars) during plotting; users explicitly call A few important points:
If you’re happy with this direction, we can also add:
Want me to expand this to cover auto-label inference and synthetic tagging when external mode is set, or leave that for a follow-up? |
|
Yes let's do this all in one go. We should do the following.
I don't think a fully external figure instance is that useful since in that case why not just use the external library? So we can skip that feature for now. |
|
Didn't mean to ping my bad! |
|
Tests pass. Just need to doc this properly. Pinged you for review @beckermr |
beckermr
left a comment
beckermr
left a comment
There was a problem hiding this comment.
A few random things. Looking great!
| return kwargs | ||
|
|
||
|
|
||
| def _inside_seaborn_call(): |
There was a problem hiding this comment.
I guess this will be a breaking change. We should bump the minor version on the next release.
beckermr
changed the title
|
pre-commit.ci autofix |
|
Something is not right with this failed test: 
|
beckermr
left a comment
beckermr
left a comment
There was a problem hiding this comment.
The image diff test failure needs addressing.
|
hmm thought I fixed that. Will address |
beckermr
left a comment
beckermr
left a comment
There was a problem hiding this comment.
I am confused by the latest commit. If an axes is marked as external, it should defer all drawing etc to the external package. Why do we need seaborn-specific code on how bars are made in ultraplot?
|
It was like that already -- not sure why either. |
|
Ah wait I see what you mean -- corrected. |
ultraplot/axes/plot.py
Outdated
| edgefix_kw = _pop_params(kw, self._fix_patch_edges) | ||
| if absolute_width is None: | ||
| absolute_width = _inside_seaborn_call() | ||
| absolute_width = False or bar_semantics == "seaborn" |
There was a problem hiding this comment.
I'm still confused by this bit. External mode should not have to know anything about seaborn at all.
The idea with the external mode is when you set an axis to "external", it should behave as if it was a standard matploitlib axis.
There are two ways to do this that I can see:
-
(what is done in this PR) Pass the ultraplot axis direcly to the other library after marking it as external and modify all of the ultraplot internals to turn itself off. In this case, the code should never need info about the settings of another library (e.g., seaborn). Instead it should put back the equivalent matplotlib defaults and let the other library deal with its own overrides. In the code above, the variable
absolute_widthshould be set to the matplotlib default when in external model. There should be no need for a separate bar configuration mode that is used to monkey patch the internals. -
In external mode, defer all calls to methods on the axis directly to the methods on the parent via something like direct invocation through the class itself (i.e.,
getattr(matplotlib.axes.Axes, <name of method>)(self, ...). IDK how feasible this approach is. It might be as simple an override of__getattribute__when in external mode to defer all attribute access to the parent.
In case 2, the PR might look like adding something like adding
def __getattribute__(self, name):
orig_attr = object.__getattribute__(self, name)
# only defer callable methods and they have to be on the parent axis
if object.__getattribute__(self, "_in_external_context")() and callable(obj_attr):
mpl_attr = getattr(matplotlib.axes.Axes, name, None)
if mpl_attr is None:
return obj_attr
else:
# we may need to use functools.partial to insert
# self at the start of the args here...
return mpl_attr
else:
return object.__getattribute__(self, name)to the class with ~no changes anywhere else. (Be careful with infinite recursion here. See https://stackoverflow.com/questions/371753/how-do-i-implement-getattribute-without-an-infinite-recursion-error.)
Does this make sense to you or did I miss something?
There was a problem hiding this comment.
Approach 2 might fail if MPL has data containers that are also themselves callable.
cvanelteren
force-pushed
the
add-seaborn-ctx
branch
from
942dab8 to
5287cdb
Compare
2 participants
Closes #405
This PR resolves spurious legend entries when plotting with seaborn on UltraPlot axes by tagging seaborn-generated helper artists (e.g., CI bands) as synthetic and excluding them during legend collection, suppressing mid-call legend/colorbar creation within seaborn to avoid duplicates, and disabling auto label inference for primary lines in seaborn contexts so legends show only hue categories unless users explicitly label additional bands.
Key changes
fill_between, error shading) as synthetic:_ultraplot_fill,_ultraplot_shade,_ultraplot_fade) unless user explicitly provides a label._ultraplot_synthetic=True.Behavior
lineplot(..., hue=...), legend now shows only hue categories.shadelabel/fadelabelorlabel=).Compatibility