Fixed colorbar bug when yscaled length is 1 #23984

Andes0113 · 2022-09-22T03:08:58Z

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).

github-actions

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.

jklymak · 2022-09-22T17:23:59Z

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

oscargus · 2022-09-29T06:28:03Z

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...

Andes0113 · 2022-09-29T17:55:20Z

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

lib/matplotlib/tests/test_colorbar.py

oscargus · 2022-10-02T10:17:14Z

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.

Andes0113 · 2022-10-02T18:09:55Z

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.

melissawm · 2023-01-22T21:37:52Z

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

Andes0113 · 2023-01-23T15:38:37Z

@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.

melissawm · 2023-01-23T16:38:59Z

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

lib/matplotlib/tests/test_colorbar.py Show resolved Hide resolved

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed colorbar bug when yscaled length is 1 #23984

Fixed colorbar bug when yscaled length is 1 #23984

Andes0113 commented Sep 22, 2022 •

edited

Loading

github-actions bot left a comment

jklymak commented Sep 22, 2022

oscargus commented Sep 29, 2022 •

edited

Loading

Andes0113 commented Sep 29, 2022

oscargus commented Oct 2, 2022

Andes0113 commented Oct 2, 2022 •

edited

Loading

melissawm commented Jan 22, 2023

Andes0113 commented Jan 23, 2023

melissawm commented Jan 23, 2023

Fixed colorbar bug when yscaled length is 1 #23984

Fixed colorbar bug when yscaled length is 1 #23984

Conversation

Andes0113 commented Sep 22, 2022 • edited Loading

PR Summary

PR Checklist

github-actions bot left a comment

Choose a reason for hiding this comment

jklymak commented Sep 22, 2022

oscargus commented Sep 29, 2022 • edited Loading

Andes0113 commented Sep 29, 2022

oscargus commented Oct 2, 2022

Andes0113 commented Oct 2, 2022 • edited Loading

melissawm commented Jan 22, 2023

Andes0113 commented Jan 23, 2023

melissawm commented Jan 23, 2023

Andes0113 commented Sep 22, 2022 •

edited

Loading

oscargus commented Sep 29, 2022 •

edited

Loading

Andes0113 commented Oct 2, 2022 •

edited

Loading