Feature: integrate pycirclize into UltraPlot #495
Conversation
cvanelteren
changed the title
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
cvanelteren
force-pushed
the
add-chord-diagram
branch
from
c1d12fb to
e0a3d1c
Compare
|
Opening this up for review. The new layout is still not stabilizing enough or the CI are too strict. The failing tests are on differences that are not caused by the code introduced here (as it is additive). |
ultraplot/axes/plot.py
Outdated
| Whether to show radial grid labels. | ||
| grid_interval_ratio : float or None, optional | ||
| Grid interval ratio (0 to 1). | ||
| grid_line_kws, grid_label_kws : dict-like, optional |
There was a problem hiding this comment.
I'd like to bring this to your attention. In ultraplot, we use a lot of _kw, like abc_kw, title_kw, rc_kw, norm_kw, which is efficient, unified, and reduces keystrokes. For these kws here, should we maintain consistency with the ultraplot style or with the pyCirclize style?
There was a problem hiding this comment.
Think you are right
There was a problem hiding this comment.
I will also move the defaults to RC while I am at it
There was a problem hiding this comment.
I really admire your efficiency. Since you're already tinkering with defaults and RC, when creating plots, there's a warning message: UltraPlotWarning: Tick label sharing not implemented for <class 'ultraplot.axes.polar.PolarAxes'> subplots. It's fine with me, and no big deal. But if you'd like take a look, maybe turn tick label sharing default to off if it's not support, and warn if users manually turn it on? If it will not complicate other thing.
There was a problem hiding this comment.
That warning is correct, to avoid it explicitly set sharing to 0. Sharing polar axes does not make sense for multiple subplots -- we may want to add a surpress flag in case it gets annoying
beckermr
left a comment
beckermr
left a comment
There was a problem hiding this comment.
Let's change kws to kw even in internal functions. Also, defaults should be in the RC.
This PR will integrated pycirclize into UltraPlot exposing chord diagrams, radar plots, and phylogeny plot. It wraps PyCirclize into an UltraPlot flavored manner adding the convenience of normal plotting while harnessing the strenghts of this well developed package.
Putting this in draft for now but it seems to work nicely with main
Closes #472