Some TernaryAxes method signatures don't reflect underlying Axes methods

morganjwilliams commented 1 year ago

Some of the methods of mpltern.ternary._axes.TernaryAxes where the behavior is inherited from mpltern.ternary._axes.TernaryAxesBase (and hence the underlying matplotlib axes class) do not share a common function signature, due to how they're redefined on mpltern.ternary._axes.TernaryAxes. This is typically not a major issue, as all associated arguments and keyword arguments are directly forwarded through - but does cause issues for introspection.

Demo

import inspect
import mpltern

inspect.signature(mpltern.ternary._axes.TernaryAxes.tripcolor)
Out: <Signature (ax, *args, **kwargs)>

inspect.signature(mpltern.ternary._axes.TernaryAxesBase.tripcolor)
Out: <Signature (ax, *args, alpha=1.0, norm=None, cmap=None, vmin=None, vmax=None, shading='flat', facecolors=None, **kwargs)>

Issue

This pops up as a bit of an issue for pyrolite which attempts to automatically determine what to forward to relevant methods based on function signatures (for better or worse), and with anonymous method signatures (i.e. where methods are re-implemented but with signatures along the lines of self, *args, **kwargs...) some keyword arguments won't get automatically forwarded.

Solutions

As this is a pyrolite issue foremost, I'm happy to solve it with a patch-on-import there, if this isn't something you'd like to modify in mpltern (in which case, feel free to close this issue).
Otherwise, I'd be happy to add a PR modifying these method signatures (and associated decorators, slightly) to reflect the underlying mostly-inherited methods.

For a PR, the two main changes would be:

to add functools.wraps to parse within the respective ternary method decorators in mpltern.ternary_parsers
to modify the signatures of methods on TernaryAxes - either by replicating the signature and docstring in these re-definitions, or by directly pointing e.g. TernaryAxes.tripcolor to TernaryAxesBase.tripcolor, while still wrapping with the appropriate decorator to handle args and the transform

yuzie007 commented 1 year ago

Thank you so much @morganjwilliams for reporting the issue! Reading the above and morganjwilliams/pyrolite#89, I think it is very reasonable to inherit the method signatures from matplotlib.

Thank you also for your suggestions to use functools.wraps (I did not know why it is necessary but now I know:)) as well as to directly point to TernaryAxesBase (thus Axes) methods. These are perfect solutions!

I have already implemented them in the master branch. Could you kindly test it by e.g.

pip install git+https://github.com/yuzie007/mpltern.git

(or something equivalent)? If it works also with pyrolite, I will publish a patch release 1.0.2 for this fix.

morganjwilliams commented 1 year ago

@yuzie007 will do, I'll let you know how it looks later today.

morganjwilliams commented 1 year ago

Can confirm that with pyrolite (as of this commit on develop, https://github.com/morganjwilliams/pyrolite/commit/d316d21adac255cb446ba67e8bb31440b88bd2db) works with the current state of mpltern/master. Outputs seem to be as expected for the tripcolor case which is the principal method used in pyrolite. Once this is released I will rollback the temporary patch I'd added on pyrolite/develop (https://github.com/morganjwilliams/pyrolite/commit/92ce2c1510deb5ba312e8216fd29b48b867f8ae1), as it will no longer be needed for the next version.

Cheers!

yuzie007 commented 1 year ago

Thank you @morganjwilliams for kind testing! I am happy to hear that the fix worked together with pyrolite. I have just now published 1.0.2, which should be already available via PyPI and will be available soon via conda-forge.

yuzie007 / mpltern