[Bug]: Validation not performed for `loc` argument to `legend` #24605

iofall · 2022-12-03T20:23:45Z

Bug summary

When passing non-str loc values to legend, validation is not performed. So even for invalid inputs, errors are raised only when we call show()

Code for reproduction

>>> import matplotlib.pyplot as plt
>>> import matplotlib as mpl
>>> xs, ys = [1,2,3], [2,3,1]
>>> fig, ax = plt.subplots(3)
>>> ax[0].scatter(xs, ys, label='loc-tuple-arg')
<matplotlib.collections.PathCollection object at 0x0000019D4099ED60>
>>> ax[0].legend(loc=(1.1, .5, 1.1, "abc"))
<matplotlib.legend.Legend object at 0x0000019D4099EF10>
>>> plt.show()

Actual outcome

Exception in Tkinter callback
Traceback (most recent call last):
  File "C:\Users\Me\anaconda3\envs\MPL\lib\tkinter\__init__.py", line 1892, in __call__
    return self.func(*args)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\tkinter\__init__.py", line 814, in callit
    func(*args)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\backends\_backend_tk.py", line 251, in idle_draw
    self.draw()
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\backends\backend_tkagg.py", line 10, in draw
    super().draw()
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\backends\backend_agg.py", line 405, in draw
    self.figure.draw(self.renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 74, in draw_wrapper
    result = draw(artist, renderer, *args, **kwargs)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 51, in draw_wrapper
    return draw(artist, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\figure.py", line 3071, in draw
    mimage._draw_list_compositing_images(
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\image.py", line 131, in _draw_list_compositing_images
    a.draw(renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 51, in draw_wrapper
    return draw(artist, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\axes\_base.py", line 3107, in draw
    mimage._draw_list_compositing_images(
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\image.py", line 131, in _draw_list_compositing_images
    a.draw(renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 51, in draw_wrapper
    return draw(artist, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\legend.py", line 641, in draw
    bbox = self._legend_box.get_window_extent(renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\offsetbox.py", line 354, in get_window_extent
    px, py = self.get_offset(w, h, xd, yd, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\offsetbox.py", line 291, in get_offset
    return (self._offset(width, height, xdescent, ydescent, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\legend.py", line 617, in _findoffset
    fx, fy = self._loc
ValueError: too many values to unpack (expected 2)
Exception in Tkinter callback
Traceback (most recent call last):
  File "C:\Users\Me\anaconda3\envs\MPL\lib\tkinter\__init__.py", line 1892, in __call__
    return self.func(*args)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\tkinter\__init__.py", line 814, in callit
    func(*args)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\backends\_backend_tk.py", line 251, in idle_draw
    self.draw()
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\backends\backend_tkagg.py", line 10, in draw
    super().draw()
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\backends\backend_agg.py", line 405, in draw
    self.figure.draw(self.renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 74, in draw_wrapper
    result = draw(artist, renderer, *args, **kwargs)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 51, in draw_wrapper
    return draw(artist, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\figure.py", line 3071, in draw
    mimage._draw_list_compositing_images(
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\image.py", line 131, in _draw_list_compositing_images
    a.draw(renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 51, in draw_wrapper
    return draw(artist, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\axes\_base.py", line 3107, in draw
    mimage._draw_list_compositing_images(
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\image.py", line 131, in _draw_list_compositing_images
    a.draw(renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\artist.py", line 51, in draw_wrapper
    return draw(artist, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\legend.py", line 641, in draw
    bbox = self._legend_box.get_window_extent(renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\offsetbox.py", line 354, in get_window_extent
    px, py = self.get_offset(w, h, xd, yd, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\offsetbox.py", line 291, in get_offset
    return (self._offset(width, height, xdescent, ydescent, renderer)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\legend.py", line 617, in _findoffset
    fx, fy = self._loc
ValueError: too many values to unpack (expected 2)

Expected outcome

Errors should be raised when invalid arguments are passed to loc. Similar to what we get when we pass an invalid string value as shown:

>>> ax[0].legend(loc="abcd")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\axes\_axes.py", line 307, in legend
    self.legend_ = mlegend.Legend(self, handles, labels, **kwargs)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\_api\deprecation.py", line 454, in wrapper
    return func(*args, **kwargs)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\legend.py", line 470, in __init__
    loc = _api.check_getitem(self.codes, loc=loc)
  File "C:\Users\Me\anaconda3\envs\MPL\lib\site-packages\matplotlib\_api\__init__.py", line 190, in check_getitem
    raise ValueError(
ValueError: 'abcd' is not a valid value for loc; supported values are 'best', 'upper right', 'upper left', 'lower left', 'lower right', 'right', 'center left', 'center right', 'lower center', 'upper center', 'center'

Additional information

Do you know why this bug is happening?

matplotlib/lib/matplotlib/legend.py

Lines 608 to 615 in ab7917a

    
           def _set_loc(self, loc): 
        
               # find_offset function will be provided to _legend_box and 
        
               # _legend_box will draw itself at the location of the return 
        
               # value of the find_offset. 
        
               self._loc_used_default = False 
        
               self._loc_real = loc 
        
               self.stale = True 
        
               self._legend_box.set_offset(self._findoffset)

No validation is done when setting values for _loc_real. We do check strings on line 473, which is why we don't face this issue there.

Operating system

Windows

Matplotlib Version

3.6.2

Matplotlib Backend

'TkAgg'

Python version

3.9.7

Jupyter version

No response

Installation

pip

The text was updated successfully, but these errors were encountered:

tacaswell · 2022-12-04T22:41:00Z

The work here is to :

sort out what the validation should be (read the code where the above traceback starts)
add logic to Legend.__init__ to validate loc
add tests
update docstring to legend (in both Legend and Axes.legend)

This is a good first issue because it should only require understanding a narrow section of the code and no API design (it is already broken for these inputs, we just want it to break better).

keyvan-dadash · 2022-12-05T09:20:15Z

Hi. can i try this?

tacaswell · 2022-12-05T15:06:54Z

@sod-lol Please do! We do not really assign issues or require you to get permission before you start working on an issue.

Gairick52 · 2023-01-09T13:25:24Z

@tacaswell hello sir can you give me some resources to work on this issue

Gairick52 · 2023-01-09T16:47:56Z

@tacaswell i want to work on this,please assign this issue to me

jklymak · 2023-01-09T16:52:45Z

@Gairick52 there is already a PR for this....

Gairick52 · 2023-01-10T13:37:14Z

@iofall Hello sir,i want to work on this issue,please assign this to me

iofall · 2023-01-10T17:49:08Z

@iofall Hello sir,i want to work on this issue,please assign this to me

Only maintainers can assign people to issues. Also, there is already a PR linked to this issue. You can try finding other issues to work on or provide any inputs if you have to the already linked PR.

Gairick52 · 2023-01-12T11:33:24Z

@iofall please share me beginner's developer's guide

iofall · 2023-01-13T16:36:14Z

@iofall please share me beginner's developer's guide

Here is the link to the contributing guide - https://matplotlib.org/devdocs/devel/contributing.html

robrighter · 2023-02-22T05:21:03Z

I picked up this ticket, continuing the previous work of @jpjepko (The new PR has him as a co-author).

I added the new check to allow the numeric mappings to string values. Because words are hard, the value error message that is raised is a bit awkward. I'm not sure what to call the valid option of a string numeric value in the error message. It is currently omitted in the error message. Let me know if anyone has a suggestion or if I should just leave it like it is.

adityasingh22 · 2023-03-10T15:55:11Z

I see this issue is still open. Does the PR #25281 not fix this? Can someone help elucidate me on it?

rcomer · 2023-03-10T16:42:06Z

Hi @adityasingh22, this issue will close when #25281 is merged.

tacaswell added Good first issue Open a pull request against these issues if there are no active ones! Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues labels Dec 4, 2022

tacaswell modified the milestones: v3.7.0, future releases Dec 4, 2022

jpjepko added a commit to jpjepko/matplotlib that referenced this issue Dec 6, 2022

matplotlib#24605 add check to validate loc tuples

bec0e34

jpjepko mentioned this issue Dec 6, 2022

Fix loc legend validation #24649

Closed

6 tasks

jpjepko added a commit to jpjepko/matplotlib that referenced this issue Dec 11, 2022

matplotlib#24605 add check to validate loc tuples

03813be

jpjepko added a commit to jpjepko/matplotlib that referenced this issue Dec 11, 2022

matplotlib#24605 add check to validate loc tuples

a7e867c

jpjepko added a commit to jpjepko/matplotlib that referenced this issue Dec 13, 2022

matplotlib#24605 add check to validate loc tuples

aacac4c

jpjepko added a commit to jpjepko/matplotlib that referenced this issue Dec 22, 2022

matplotlib#24605 add check to validate loc tuples

5b5d17d

jpjepko added a commit to jpjepko/matplotlib that referenced this issue Dec 22, 2022

matplotlib#24605 add check to validate loc tuples

0b7fcba

rcomer linked a pull request Dec 23, 2022 that will close this issue

Fix loc legend validation #24649

Closed

6 tasks

QuLogic modified the milestones: future releases, v3.7.0 Jan 5, 2023

QuLogic modified the milestones: v3.7.0, v3.8.0 Jan 10, 2023

robrighter mentioned this issue Feb 22, 2023

Bugfix for loc legend validation #25281

Merged

6 tasks

rcomer closed this as completed in #25281 Mar 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: Validation not performed for `loc` argument to `legend` #24605

[Bug]: Validation not performed for `loc` argument to `legend` #24605

iofall commented Dec 3, 2022

tacaswell commented Dec 4, 2022

keyvan-dadash commented Dec 5, 2022

tacaswell commented Dec 5, 2022

Gairick52 commented Jan 9, 2023

Gairick52 commented Jan 9, 2023

jklymak commented Jan 9, 2023

Gairick52 commented Jan 10, 2023

iofall commented Jan 10, 2023

Gairick52 commented Jan 12, 2023

iofall commented Jan 13, 2023

robrighter commented Feb 22, 2023

adityasingh22 commented Mar 10, 2023

rcomer commented Mar 10, 2023

[Bug]: Validation not performed for loc argument to legend #24605

[Bug]: Validation not performed for loc argument to legend #24605

Comments

iofall commented Dec 3, 2022

Bug summary

Code for reproduction

Actual outcome

Expected outcome

Additional information

Operating system

Matplotlib Version

Matplotlib Backend

Python version

Jupyter version

Installation

tacaswell commented Dec 4, 2022

keyvan-dadash commented Dec 5, 2022

tacaswell commented Dec 5, 2022

Gairick52 commented Jan 9, 2023

Gairick52 commented Jan 9, 2023

jklymak commented Jan 9, 2023

Gairick52 commented Jan 10, 2023

iofall commented Jan 10, 2023

Gairick52 commented Jan 12, 2023

iofall commented Jan 13, 2023

robrighter commented Feb 22, 2023

adityasingh22 commented Mar 10, 2023

rcomer commented Mar 10, 2023

[Bug]: Validation not performed for `loc` argument to `legend` #24605

[Bug]: Validation not performed for `loc` argument to `legend` #24605