NCAR / geocat-viz

GeoCAT-viz contains tools to help plot geoscience data, including convenience and plotting functions that are used to facilitate plotting geosciences data with Matplotlib, Cartopy, and other visualization packages.
https://geocat-viz.readthedocs.io
Other
48 stars 19 forks source link

Taylor diagram issue when setting annotate_on to False #206

Closed srosanka closed 8 months ago

srosanka commented 8 months ago

Describe the bug When working with Taylor diagrams and setting annotate_on to False in add_model_set the code breaks at the return statement of add_model_set since the list modelTexts is undefined. In addition, the list modelTexts will be undefined for plotting outlier model stats.

To Reproduce Set annotate_on to False in add_model_set.

Fix/Solution This bug can be easily fixed by moving the definition of modelTexts outside the if annotate_on statement. I.e., change the code from:

# Annotate model markers if annotate_on is True
if annotate_on:
     # Initialize empty array that will be filled with model label text objects and returned
     modelTexts = []

     for std, corr in zip(std_plot, corr_plot):
     label = str(stdAndNumber[std])
     textObject = self.ax.annotate(label, (np.arccos(corr), std),
                                                    fontsize=fontsize,
                                                    color=color,
                                                    textcoords="offset pixels",
                                                    xytext=xytext)
     modelTexts.append(textObject)

to:

# Initialize empty array that will be filled with model label text objects and returned
modelTexts = []

# Annotate model markers if annotate_on is True
if annotate_on:
     for std, corr in zip(std_plot, corr_plot):
     label = str(stdAndNumber[std])
     textObject = self.ax.annotate(label, (np.arccos(corr), std),
                                                    fontsize=fontsize,
                                                    color=color,
                                                    textcoords="offset pixels",
                                                    xytext=xytext)
     modelTexts.append(textObject)

I already implemented this fix in my Fork and I will open a corresponding Pull request.

kafitzgerald commented 8 months ago

Thanks so much for the report!

A pull request would be great!

srosanka commented 8 months ago

@kafitzgerald I just marked the related pull request as ready for review. Please let me know if anything is unclear.