New functions for plotting Taylor diagrams in subplots

[adlzanchetta]

Added functions that allows the plotting of Taylor diagrams in subplots. These functions are very similar to the previously existing ones, with the support for the additional arguments:

• colFrame: Color for both the y (left) and x (bottom) spines;
• markerColors: Two color values for the markers (face and edge);
• titleCORshape: Shape of the label "correlation coefficient" ("curved", "linear") ;
• colsCOR: Three independent values for the correlation coefficient graph components (grid, tick_labels, title);
• colsSTD: Three independent values for the standard deviation graph components (grid, tick_labels, title).

Only Taylor diagram markers are supported. Taylor diagram colorbar and Target diagram not yet supported.

Note: Commented reference to get_axis_tick_label() in plot_taylor_axes.py as the file get_axis_tick_label.py seems to be missing.

[PeterRochford]

Andre,

I apologize for this lengthy but there is much to discuss.

Thank you for your contribution. I applaud your enthusiasm and perseverance to customize a multi-panel plot of the Taylor diagrams. You are more current than I on the latest Python features and I am jealous you have the time to devote to such a pursuit.

Your pull request involves the most extensive code changes to the SkillMetrics package I've encountered to date. So I will be doing a code review that can be accessed via the "Files Changed" tab above. The code you are providing would be a significant upgrade of the package that would allow for greater customization of multi-panel plots of Taylor diagrams that could also be adapted for target diagrams. However, the challenge is how to incorporate your contributions so that the following two criteria can be satisfied:

1. The upgrade does not break any programs using the package, i.e. all example Taylor scripts still work.
2. The upgrade does not result in duplication of code that would invite maintenance problems or lead to counterproductive complexity over time.

Your pull request satisfies the first item, but as your remark above, the contributed functions are very similar to the previously existing ones which conflicts with the second item. My thought as to the best solution is to implement the method overloading that is available in Python 3 (see for example Method Overloading in Python), which is something I have never done. With some investigation, I think it's possible to create a taylor_diagram.py that contains two methods, one for 3 arguments that supports the current use and one with 4 arguments that supports the subplot control features you desire. The former would define the needed ax variable and then call the latter, thereby eliminating unnecessary duplication. It would look something like:

from multidispatch import dispatch 

@dispatch(type1, type2, type3)  # using the dispatch decorator to define 3 parameters of appropriate types
def taylor_diagram_subplot(stds=None, rmss=None, cors=None, **kwargs):
   ax = ...
   taylor_diagram(ax, stds, rmss, cors, **kwargs)

@dispatch(type1, type2, type3, type4)  # using the dispatch decorator to define 4 parameters of appropriate types
def taylor_diagram(ax=None, stds=None, rmss=None, cors=None, **kwargs):
  ...

Note I haven't researched what should be specified as type1, type2, etc. I can test the 3-parameter implementation with my regression test suite of 13 Taylor examples once a prototype is available.

Our biggest challenge will be in writing a taylor_diagram function that supports method overloading. If we are successful, I think most of the functions in the skill_metrics folder of your pull request can be direct replacements for the parent functions from which they originated. Any missing capabilities should require only slight modifications. The taylor14.py example would only require calling taylor_diagram instead. This would result in a much cleaner and tightly written package.

If you are in favor of this suggestion I'll take the time to provide feedback in the code review as cited above. We can discuss collaborating on creating the overloaded taylor_diagram method. Please let me know your thoughts on how you would like to proceed.

Thanks, Peter

[adlzanchetta]

Peter,

I totally agree that the way was not the best way to organize the code in terms of maintainability. The solution you proposed seems interesting, but I am not very familiar with method overloading in Python (thanks for pointing out, by the way).

My original idea was to simply add one optional argument ax to the function taylor_diagram() and add some local if / else statements for the case a value was given (or not). So the original function:

def target_diagram(*args, **kwargs):
    ...
    cax = plt.gca()
    ...

would become:

def target_diagram(ax = None, *args, **kwargs):
    ...
    cax = plt.gca() if ax is None else ax
    ...

and make the appropriate adjustments, replacing line like:

    plt.xlim(...)

to

    cax.set_xlim(...)

It would not follow strictly the current approach of passing arguments (only through *args and **kargs), but I imagine it would disturb less the current implementation than the method overloading.

How do you see it?

I don't want to take your time with it, so if you agree with this I can make these adjustments and submit a new Pull Request that will have only one new file with a standalone function and a handful of lines added/modified.

Thanks, Andre.

[adlzanchetta]

Dear Peter,

I really apologize for the long silence. Some turbulence on my side took away my attention. Hopefully I can now reconnect and wrap up this subject.

I think I didn't receive your email. If you have already sent it, to which address was it?

I don't mind getting or not credit for the submission, but it may be useful to have it there so people can blame me in the feature in the case of a bug is found. If you want to commit the code yourself, I am 100% fine with it. Whichever you prefer.

I just think it could be interesting to update the package in pypi. The last version available there is from Out 2021. It seems that the current master branch is already a few commits ahead what is there, and for new users it can be more appealing to explore a package that was recently updated (even if the update itself does not bring tons of new things).

Bests, Andre.

[PeterRochford]

Andre,

Here are the changed python files I mentioned in the comments to your pull request. Hopefully, everything will work flawlessly.

Let me know if you encounter any problems.

Peter

On Aug 16, 2022, at 4:53 PM, Andre Della Libera Zanchetta @.***> wrote:

Peter,

I totally agree that the way was not the best way to organize the code in terms of maintainability. The solution you proposed seems interesting, but I am not very familiar with method overloading in Python (thanks for pointing out, by the way).

My original idea was to simply add one optional argument ax to the function taylor_diagram() and add some local if / else statements for the case a value was given (or not). So the original function:

def target_diagram(*args, **kwargs): ... cax = plt.gca() ... would become:

def target_diagram(ax = None, *args, **kwargs): ... cax = plt.gca() if ax is None else ax ... and make the appropriate adjustments, replacing line like:

plt.xlim(...)

to

cax.set_xlim(...)

It would not follow strictly the current approach of passing arguments (only through *args and **kargs), but I imagine it would disturb less the current implementation than the method overloading.

How do you see it?

I don't want to take your time with it, so if you agree with this I can make these adjustments and submit a new Pull Request that will have only one new file with a standalone function and a handful of lines added/modified.

Thanks, Andre.

— Reply to this email directly, view it on GitHub https://github.com/PeterRochford/SkillMetrics/pull/28#issuecomment-1217153210, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF3YS7AGUF3Q26IQRPFETV3VZP5TVANCNFSM56QABWVA. You are receiving this because you commented.