-
Notifications
You must be signed in to change notification settings - Fork 51
Description
Summary
The contrast_ylim argument currently has no effect when using matplotlib.
A previous discussion (#128) mentioned that supporting contrast_ylim was considered not feasible at that time due to automatic calculation of contrast limits.
What I found
While reviewing the implementation, I noticed that the gardner_altman_adjustments function always computes its own contrast axis limits and does not appear to prioritize a user-specified contrast_ylim.
I understand that the current design may have historical or technical reasons behind it.
However, I wonder if it might be beneficial to allow a user-provided contrast_ylim to override the automatically computed limits when explicitly set.
Proposed solution
I would like to propose adding logic such that:
- If
contrast_ylimis provided by the user, it takes priority. - Otherwise, the current automatic behavior remains unchanged.
Below is a simplified example of how the logic could be applied:
if effect_size_type in ["mean_diff", "median_diff"]:
if contrast_ylim is not None:
# Respect user-specified y-limits.
contrast_axes.set_ylim(contrast_ylim)
else:
# Current automatic behavior.
rawdata_ylims = np.array(rawdata_axes.get_ylim())
if current_effsize > 0:
rightmin, rightmax = rawdata_ylims - current_effsize
elif current_effsize < 0:
rightmin, rightmax = rawdata_ylims + current_effsize
else:
rightmin, rightmax = rawdata_ylims
contrast_axes.set_ylim(rightmin, rightmax)
# The remaining lines follow the existing implementation.
og_ylim_contrast = rawdata_axes.get_ylim() - np.array(control_group_summary)
contrast_axes.set_ylim(og_ylim_contrast)
contrast_axes.set_xlim(contrast_xlim_max - 1, contrast_xlim_max)Expected behavior
When contrast_ylim is given:
- The contrast axes would respect the user-defined y-limits.
- This would provide behavior consistent with the plotly implementation, which already supports
contrast_ylim.
Actual behavior
Currently:
- In Gardner–Altman plots, the
contrast_ylimargument is not taken into account. - The axes limits are determined solely by automatic calculations.
Additional context
If maintainers feel that this direction is reasonable,
I would be happy to submit a Pull Request including:
- The proposed patch
- Tests reflecting the updated behavior
Thank you for considering this, and please let me know your thoughts.