Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some TernaryAxes method signatures don't reflect underlying Axes methods #13

Closed
morganjwilliams opened this issue Aug 25, 2023 · 4 comments

Comments

@morganjwilliams
Copy link

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
Copy link
Owner

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
Copy link
Author

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

@morganjwilliams
Copy link
Author

Can confirm that with pyrolite (as of this commit on develop, morganjwilliams/pyrolite@d316d21) 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 (morganjwilliams/pyrolite@92ce2c1), as it will no longer be needed for the next version.

Cheers!

@yuzie007
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants