BUG: Fix broken call to ax.legend

[wnavarre]

It seems we are getting tripped up by ax.legend's rather complex calling convention, resulting in an error: "You have mixed positional and keyword arguments, some input may be discarded."

Simplify the call by integrating all arguments into kwargs.

[wnavarre]

Tests failed, but it seems possible they weren't caused by my commit since the master branch is also failing.

I rebased off of the commit described below. This seems to be the most recent commit that passed CI on master.

commit e32573d
Author: NoharaMasato 38820014+NoharaMasato@users.noreply.github.com
Date: Tue Apr 25 11:30:59 2023 +0900

[martinfleis]

It seems we are getting tripped up by ax.legend's rather complex calling convention, resulting in an error: "You have mixed positional and keyword arguments, some input may be discarded."

Where do you see this error? Do you have a reproducible example?

The fix looks okay to me but I'd like to see the bug first :).

[wnavarre]

Yes. Bug occurs with when using "labels" in the legend_kwds dict argument to plot. Example code below.

The output in the terminal for this code is:

/home/navarre/micromamba/envs/geo_env/lib/python3.8/site-packages/geopandas/plotting.py:944: UserWarning: You have mixed positional and keyword arguments, some input may be discarded.
  ax.legend(patches, categories, **legend_kwds)
See the problem?
Better?

And the failing and succeding maps (note the weird little box in the upper left of the failing map):

import geopandas
import matplotlib.pyplot as plt

path_to_data = geopandas.datasets.get_path("nybb")
gdf = geopandas.read_file(path_to_data)

def go(ax):
    gdf.plot(column="BoroName",
             ax=ax,
             categorical=True,
             categories=["Manhattan",
                         "Bronx",
                         "Staten Island",
                         "Brooklyn",
                         "Queens"],
             legend=True,
             legend_kwds= {
                 "labels":["New York County",
                           "Bronx County",
                           "Richmond County",
                           "Kings County",
                           "Queens County"],
                 "loc": "upper left"
             })

fig, ax = plt.subplots(figsize = (10,10))
go(ax)
fig.show()
input("See the problem?")

# You've seen the problem. Now for demonstrating the solution:
    
def make_custom_legend_method(ax):
    normal_method = ax.legend
    def proposed_fix(*args, **kwargs):
        if len(args) != 2:
            # The proposed fix is on a callsite where args always equaled two...
            # Just behave as normal.
            return normal_method(*args, **kwargs)
        handles, labels = args
        kwargs.setdefault("handles", handles)
        kwargs.setdefault("labels", labels)
        normal_method(**kwargs)
    return proposed_fix

fig, ax = plt.subplots(figsize = (10,10))
ax.legend = make_custom_legend_method(ax)
go(ax)
fig.show()
input("Better?")

# Better?

[martinfleis]

Ah! Great, thanks!

We should probably also add a test for this case as this apparent failure was not caught by our current test suite. Could you add one? There'll be some other testing the legend, so you can mirror the implementation.

[wnavarre]

Ok. Edited a test case to hit this problem.

[martinfleis]

Thanks! the CI failure is not related.