NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.4k

Make the filternorm prop of Images a boolean rather than a {0,1} scalar.#10442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

QuLogic merged 1 commit intomatplotlib:masterfromanntzer:filternorm

Apr 11, 2018

Merged

Make the filternorm prop of Images a boolean rather than a {0,1} scalar.#10442

QuLogic merged 1 commit intomatplotlib:masterfromanntzer:filternorm

Apr 11, 2018

Conversation

Copy link

Contributor

anntzer commentedFeb 13, 2018

This seems a bit more pythonic and the C-API handles bools just as well.
Also filternorm and filterrad are always defined (as bool and float,
respectively, so no need to add... or 0.0 when passing them to the
C-API.

PR Summary

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

anntzer added this to thev3.0 milestone

Feb 13, 2018

anntzer force-pushed thefilternorm branch from9c6342d to11495e9Compare

February 13, 2018 19:08

Copy link

Member

QuLogic commentedFeb 13, 2018

A little bit of this is part of#10383.

Copy link

ContributorAuthor

anntzer commentedFeb 13, 2018

seems easy to rebase in either direction

Make the filternorm prop of Images a boolean rather than a {0,1} scalar.

8e68152

This seems a bit more pythonic and the C-API handles bools just as well.Also filternorm and filterrad are always defined (as bool and float,respectively, so no need to add `... or 0.0` when passing them to theC-API.

anntzer force-pushed thefilternorm branch from11495e9 to8e68152Compare

April 8, 2018 00:35

anntzer mentioned this pull request

Apr 8, 2018

Improve docstring of Axes.imshow#10982

Merged

jklymak reviewed

Apr 8, 2018

View reviewed changes

lib/matplotlib/axes/_axes.py

		filternorm : scalar, optional, default: 1
		A parameter for the antigrain image resize filter. From the
		antigrain documentation, if `filternorm` = 1, the filter
		filternorm : bool, optional, default: True

Copy link

Member

jklymakApr 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

OK, sofilternorm was either 1 or zero?

The rest of this docstring doesn't make a lick of sense to me. I think they are trying to say the sum of the filter should be one, but???

Copy link

ContributorAuthor

anntzerApr 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Feel free to propose a better wording :-) (it doesn't make sense to me either)
At least since#10383matplotlib._image.resample also documentsnorm as a bool.

Copy link

Member

timhoffm commentedApr 8, 2018•
edited
Loading

Concerning removal of... or 0.0:

What if users have usedfilterrad=False orfilterrad=0? Until now, theor has silently cast them to float. Do it still work? At least the second one should.

Copy link

ContributorAuthor

anntzer commentedApr 8, 2018

set_filterrad has a check:

    def set_filterrad(self, filterrad)        """```        Set the resize filter radius only applicable to some        interpolation schemes -- see help for imshow        ACCEPTS: positive float        """        r = float(filterrad)        if r <= 0:            raise ValueError("The filter radius must be a positive number")        self._filterrad = r        self.stale = True

so the proposed case cannot occur.