You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hi @raphaelvallat , I'm wondering how tied you are to generating figures explicitly in most of pingouin's plotting functions (e.g., plot_blandaltman). Currently, most of these plotting functions generate a figure explicitly when the ax parameter is not provided, something like:
I think it might be better or more flexible to generate a figure with default size the way seaborn does:
ifaxisNone:
ax=plt.gca()
Seems trivial, but if it's set up this way, then the function can be used to "map" plots onto seaborn grids (e.g., this demo). It would be great if pingouin's plotting functions could be applied to this popular seaborn feature. Other very minor changes would also need to happen, namely taking a color parameter (see here).
Other reasons to favor this change:
As far as I can tell, you don't lose anything except the ability to specify figure details. BUT that's where the ax parameter gets its use; if someone wants a figure a specific size, they open up the figure themselves (specifiying size, dpi, etc.) and then pass the ax in. Ultimately this has more complete flexibility and suggests the user build their own figure if they are concerned about aesthetic details.
Currently, when ax is used, figsize (and and sometimes dpi) is ignored. This is slightly confusing so this avoids that as well.
This would of course not apply to any figures that need to build their own, like anything using seaborn facetgrids. At a glance, I think this change could apply to the following functions:
plot_blandaltman
plot_paired
plot_circmean
qqplot (also dropping the automatic Title here would help compatibility with seaborn)
Note there is a 3rd option where you could have both. You could check if a figure already exists, and then if not build one with the arguments passed to the pingouin plotting function. I don't favor this much, but just mentioning it.
If you're into this I'd be happy to make the pull request.
The text was updated successfully, but these errors were encountered:
Thanks for the detailed issue. I don't see any reasons why not to implement your proposed solution, and I agree that it may improve flexibility and at the same time simplify Pingouin's API. Please feel free to work on a PR 👍
Hi @raphaelvallat , I'm wondering how tied you are to generating figures explicitly in most of pingouin's plotting functions (e.g.,
plot_blandaltman
). Currently, most of these plotting functions generate a figure explicitly when theax
parameter is not provided, something like:I think it might be better or more flexible to generate a figure with default size the way seaborn does:
Seems trivial, but if it's set up this way, then the function can be used to "map" plots onto seaborn grids (e.g., this demo). It would be great if pingouin's plotting functions could be applied to this popular seaborn feature. Other very minor changes would also need to happen, namely taking a
color
parameter (see here).Other reasons to favor this change:
ax
parameter gets its use; if someone wants a figure a specific size, they open up the figure themselves (specifiying size, dpi, etc.) and then pass the ax in. Ultimately this has more complete flexibility and suggests the user build their own figure if they are concerned about aesthetic details.ax
is used,figsize
(and and sometimesdpi
) is ignored. This is slightly confusing so this avoids that as well.This would of course not apply to any figures that need to build their own, like anything using seaborn facetgrids. At a glance, I think this change could apply to the following functions:
plot_blandaltman
plot_paired
plot_circmean
qqplot
(also dropping the automatic Title here would help compatibility with seaborn)Note there is a 3rd option where you could have both. You could check if a figure already exists, and then if not build one with the arguments passed to the pingouin plotting function. I don't favor this much, but just mentioning it.
If you're into this I'd be happy to make the pull request.
The text was updated successfully, but these errors were encountered: