FIX #3537 Disabling the legend based on the x-axis and hue values

[abdulwaheedsoudagar]

Disable legend in case of redundancy
sns.catplot(tips, x="day", y="total_bill", hue="day", col="sex", row="smoker", kind="box", height=3)

[mwaskom]

Thanks for taking a crack at this, but it's not a sufficient change. First, that simple comparison will raise if any of the variables are specified using vector data (i.e. pandas series or numpy arrays, where the == operator works element-wise and returns a vector). Second, we want to continue supporting a (redundant) legend when the user asks for it explicitly by passing legend=True. You'll want to take a look at how this is currently handled in the axes-level functions, i.e. in this method:

seaborn/seaborn/categorical.py

Line 411 in 863539d

def _configure_legend(self, ax, func, common_kws=None, semantic_kws=None):

[abdulwaheedsoudagar]

Thanks for taking a crack at this, but it's not a sufficient change. First, that simple comparison will raise if any of the variables are specified using vector data (i.e. pandas series or numpy arrays, where the == operator works element-wise and returns a vector). Second, we want to continue supporting a (redundant) legend when the user asks for it explicitly by passing legend=True. You'll want to take a look at how this is currently handled in the axes-level functions, i.e. in this method:

seaborn/seaborn/categorical.py

Line 411 in 863539d

def _configure_legend(self, ax, func, common_kws=None, semantic_kws=None):

Thank you for responding. I have a brief inquiry. The problem of the legend not showing up occurs when the setting is "legend = auto." Yet, when it's set to "legend = full" or "legend = brief," it appears correctly, when hue and x values are same. Should the legend only be displayed when it's set to "True", or also when it's "full" or "brief"?

[mwaskom]

Should the legend only be displayed when it's set to "True", or also when it's "full" or "brief"?

"True" (as a string type) is not a valid argument for legend but True (as a boolean type) is. The only place where we want conditional logic about a redundant legend is when legend="auto". Otherwise it should appear when requested (with True, "brief", or "full") and be suppressed when not (with False).

[abdulwaheedsoudagar]

Should the legend only be displayed when it's set to "True", or also when it's "full" or "brief"?

"True" (as a string type) is not a valid argument for legend but True (as a boolean type) is. The only place where we want conditional logic about a redundant legend is when legend="auto". Otherwise it should appear when requested (with True, "brief", or "full") and be suppressed when not (with False).

Thanks, Based on your comment I have modified the code, legend title is disabled based on x and y values and hue datatype.
In function _configure_legend, legend title is not decided, it is decieded in line 3098, so I added conditions for this https://github.com/abdulwaheedsoudagar/seaborn/blob/d4ec331b57a58287b05cc49191b4b5b54797118f/seaborn/categorical.py#L3098 statement.

[codecov]

Codecov Report

Merging #3540 (79dc7d5) into master (863539d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3540   +/-   ##
=======================================
  Coverage   98.34%   98.34%           
=======================================
  Files          75       75           
  Lines       24605    24613    +8     
=======================================
+ Hits        24197    24205    +8     
  Misses        408      408           

Files	Coverage Δ
seaborn/categorical.py	98.91% <100.00%> (+<0.01%)	⬆️
tests/test_categorical.py	99.25% <100.00%> (+<0.01%)	⬆️

[mwaskom]

I don't think you don't need all of this complexity; as in _configure_legend you can take advantage of the _redundant_hue property to cover the edge case.

Additionally, the conditional should guard the entire add_legend call, not just what the title gets set to. The plot should not have a legend when legend="auto" and the hue variable is redundant.

[abdulwaheedsoudagar]

Initially, I started with what you suggested and found that in the current implementation when the x and hue columns are the same _redundant_hue is set as True and show_legend as False, which causes the legend data not to display, but the legend column is still displayed. so I went with modifying the title

Also if I comment the if show_legend code in function _configure_legend, even then legend title(that is day in this example) will be displayed. what I understood is in this function legend data is decided but not title

def _configure_legend(self, ax, func, common_kws=None, semantic_kws=None):

    if self.legend == "auto":
        show_legend = not self._redundant_hue and self.input_format != "wide"
    else:
        show_legend = bool(self.legend)

    if show_legend:
        self.add_legend_data(ax, func, common_kws, semantic_kws=semantic_kws)
        handles, _ = ax.get_legend_handles_labels()
        if handles:
            ax.legend(title=self.legend_title)

[mwaskom]

I am not suggesting you modify _configure_legend, I am suggesting you base the changes to the body of catplot off of it. I think you just need the following patch

diff --git a/seaborn/categorical.py b/seaborn/categorical.py
index bb40d5d3..be68617a 100644
--- a/seaborn/categorical.py
+++ b/seaborn/categorical.py
@@ -3095,7 +3095,12 @@ def catplot(
         g._update_legend_data(ax)
         ax.legend_ = None
 
-    if legend and "hue" in p.variables and p.input_format == "long":
+    if legend == "auto":
+        show_legend = not p._redundant_hue and p.input_format != "wide"
+    else:
+        show_legend = bool(legend)
+
+    if show_legend:
         g.add_legend(title=p.variables.get("hue"), label_order=hue_order)
 
     if data is not None:

There is a bit of copy-paste from _configure_legend that could perhaps get abstracted away, but it's not a big deal.

[abdulwaheedsoudagar]

Ah, I overlooked that the class p contains the variable _redundant_hue. I've made the appropriate changes. Thanks

[mwaskom]

Can you please add a test? Essentially you want to assert that the FacetGrid returned by catplot under this condition has a null legend attribute.

[abdulwaheedsoudagar]

Can you please add a test? Essentially you want to assert that the FacetGrid returned by catplot under this condition has a null legend attribute.

Sure, I will add

[mwaskom]

Thanks @abdulwaheedsoudagar