Fixed colorbar bug when yscaled length is 1 by Andes0113 · Pull Request #23984 · matplotlib/matplotlib · GitHub

PR Summary

This pull request aims to solve issue #23817 in which an error would be thrown in the event that yscaled's length was 1. To fix this, I added a check to see if yscaled's length was 1, and if so, to set automin and automax to yscaled's only value. I also moved the declaration of automin and automax inside the if statement where extendlength is declared as suggested by @oscargus , since they are unused otherwise.

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

[N/A] New features are documented, with examples if plot related.
[N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
[N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
[N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

Can you add a test that doesn't work on main, but is fixed by this PR? Probably in tests/test_colorbar.py.

As there is now a warning, which is clearly better than an error, you will need to take that intro consideration in the test.

Something like:

with pytest.warns(UserWarning) as record:
    ax.contour([[1, 1], [1, 1]])
assert len(record) == 1

should do. Also, please add a comment like:

# Smoke test for gh#23817

at the beginning of the test (so that it is clear what is tested as there is "no assert" (the assert above is really not crucial for the test purpose)).

Edit: the warning was probably always there, but earlier "hidden" by the error, so the first sentence is not completely correct...

Thanks for the help! I've added the changes, and I'll circle back later to see if all these tests pass.

It seems like there is a divide by zero(?) elsewhere in the code now:

lib/matplotlib/tests/test_colorbar.py:251: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
lib/matplotlib/figure.py:1276: in colorbar
    cb = cbar.Colorbar(cax, mappable, **cb_kw)
lib/matplotlib/_api/deprecation.py:384: in wrapper
    return func(*inner_args, **inner_kwargs)
lib/matplotlib/colorbar.py:396: in __init__
    self._draw_all()
lib/matplotlib/colorbar.py:535: in _draw_all
    X, Y = self._mesh()
lib/matplotlib/colorbar.py:1110: in _mesh
    y, _ = self._proportional_y()

here

matplotlib/lib/matplotlib/colorbar.py

Lines 1223 to 1225 in cb9ae92

    
           y = (self._boundaries - self._boundaries[self._inside][0]) 
        
           y = y / (self._boundaries[self._inside][-1] - 
        
                    self._boundaries[self._inside][0])

One should probably check something there as well? I do not know if the length of _boundaries[self._inside] is one or the first and last element is the same.

One should probably check something there as well? I do not know if the length of _boundaries[self._inside] is one or the first and last element is the same.

We could probably just put in a check like this:
y = (self._boundaries - self._boundaries[self._inside][0])
if self._boundaries[self._inside][-1] != self._boundaries[self._inside][0]:
` y = y / (self._boundaries[self._inside][-1] - self._boundaries[self._inside][0])
And it would work for both cases. I am a little worried about this causing unexpected behavior though, but it would likely be only in this case anyway.

Another idea:

The boundaries are set in _process_values(), here:

matplotlib/lib/matplotlib/colorbar.py

Lines 1050 to 1055 in 1b3019d

    
               def _process_values(self): 
        
                   """ 
        
                   Set `_boundaries` and `_values` based on the self.boundaries and 
        
                   self.values if not None, or based on the size of the colormap and 
        
                   the vmin/vmax of the norm. 
        
                   """

Not sure which section of this function we're setting the boundaries in for this case though.

I imagine it would be here:

matplotlib/lib/matplotlib/colorbar.py

Lines 1056 to 1068 in 1b3019d

    
           if self.values is not None: 
        
               # set self._boundaries from the values... 
        
               self._values = np.array(self.values) 
        
               if self.boundaries is None: 
        
                   # bracket values by 1/2 dv: 
        
                   b = np.zeros(len(self.values) + 1) 
        
                   b[1:-1] = 0.5 * (self._values[:-1] + self._values[1:]) 
        
                   b[0] = 2.0 * b[1] - b[2] 
        
                   b[-1] = 2.0 * b[-2] - b[-3] 
        
                   self._boundaries = b 
        
                   return 
        
               self._boundaries = np.array(self.boundaries) 
        
               return

though, since the catch for checking if self.boundaries is None would not catch the case where it would have length 1. In that case, maybe we should add a check there instead of when dividing the y by the ends of the boundaries? That way we would still have valid boundaries for later calculations.

I'll try the change that you mentioned for now, though.

Hi @Andes0113 - is this ready for a second round of reviews?

@melissawm Think this PR is ready to be closed since there are branching PRs that attempt to do the same thing and are further along in the process. I'd take a look at the more recent related PRs #24656 and #24698 because I've stopped working on this issue.

Thanks for the update @Andes0113 - hope to see you again around Matplotlib!

Fixed colorbar bug when yscaled length is 1

0858774

github-actions Bot reviewed Sep 22, 2022

View reviewed changes

Andes0113 mentioned this pull request Sep 22, 2022

[Bug]: Error showing colorbar when contouring a uniform field #23817

Closed

Andes0113 added 2 commits September 22, 2022 15:08

Added test

1f2418a

Fixed flake issue with test

3338d20

oscargus linked an issue Sep 29, 2022 that may be closed by this pull request

[Bug]: Error showing colorbar when contouring a uniform field #23817

Closed

oscargus added the topic: color/colorbar label Sep 29, 2022

Added smoke test for gh#23817

b99e758

Added smoke test for gh#23817

b2e5dfa

oscargus reviewed Sep 30, 2022

View reviewed changes

Comment thread lib/matplotlib/tests/test_colorbar.py

Fixed uniformfield test

501c00a

timhoffm mentioned this pull request Oct 2, 2022

Handles yscaled with length 1 #24076

Closed

6 tasks

Added check for contoured uniform fields

5e581f9

quinnah mentioned this pull request Dec 7, 2022

[WIP] Attempted fix to colorbar bug when yscaled len = 1 as suggested in previous PR #24656

Draft

6 tasks

melissawm closed this Jan 23, 2023

Fixed colorbar bug when yscaled length is 1 by Andes0113 · Pull Request #23984 · matplotlib/matplotlib · GitHub

Repository navigation

Fixed colorbar bug when yscaled length is 1 - #23984

Fixed colorbar bug when yscaled length is 1#23984
Andes0113 wants to merge 7 commits into
matplotlib:mainfrom
Andes0113:ColorBar-YScale-BugFix

Andes0113 commented Sep 22, 2022 •
edited
Loading

Uh oh!

Uh oh!

github-actions Bot left a comment

Uh oh!

jklymak commented Sep 22, 2022

Uh oh!

oscargus commented Sep 29, 2022 •
edited
Loading

Uh oh!

Uh oh!

Andes0113 commented Sep 29, 2022

Uh oh!

Uh oh!

oscargus commented Oct 2, 2022

Uh oh!

Andes0113 commented Oct 2, 2022 •
edited
Loading

Uh oh!

Uh oh!

melissawm commented Jan 22, 2023

Uh oh!

Andes0113 commented Jan 23, 2023

Uh oh!

melissawm commented Jan 23, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fixed colorbar bug when yscaled length is 1 by Andes0113 · Pull Request #23984 · matplotlib/matplotlib · GitHub

Repository navigation

Fixed colorbar bug when yscaled length is 1 - #23984

Fixed colorbar bug when yscaled length is 1#23984Andes0113 wants to merge 7 commits intomatplotlib:mainmatplotlib/matplotlib:mainfrom Andes0113:ColorBar-YScale-BugFixAndes0113/matplotlib:ColorBar-YScale-BugFixCopy head branch name to clipboard

Conversation

Andes0113 commented Sep 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jklymak commented Sep 22, 2022

Uh oh!

oscargus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andes0113 commented Sep 29, 2022

Uh oh!

Uh oh!

oscargus commented Oct 2, 2022

Uh oh!

Andes0113 commented Oct 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melissawm commented Jan 22, 2023

Uh oh!

Andes0113 commented Jan 23, 2023

Uh oh!

melissawm commented Jan 23, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fixed colorbar bug when yscaled length is 1#23984
Andes0113 wants to merge 7 commits into
matplotlib:mainfrom
Andes0113:ColorBar-YScale-BugFix

Andes0113 commented Sep 22, 2022 •
edited
Loading

oscargus commented Sep 29, 2022 •
edited
Loading

Andes0113 commented Oct 2, 2022 •
edited
Loading