Skip to content

API: rename DivergingNorm to TwoSlopeNorm #15450

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 20, 2019
Merged

API: rename DivergingNorm to TwoSlopeNorm #15450

merged 2 commits into from
Oct 20, 2019

Conversation

tacaswell
Copy link
Member

@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Oct 19, 2019
@tacaswell tacaswell added this to the v3.2.0 milestone Oct 19, 2019
@efiring
Copy link
Member

efiring commented Oct 20, 2019

Docs build warnings -> failure:

~ $ grep WARNING build_25733_step_107_container_0.txt
/home/circleci/project/doc/tutorials/colors/colormapnorms.rst:246: WARNING: Title underline too short.
/home/circleci/project/doc/tutorials/colors/colormapnorms.rst:246: WARNING: Title underline too short.
/home/circleci/project/lib/matplotlib/colors.py:docstring of matplotlib.colors.DivergingNorm:1: WARNING: py:class reference target not found: matplotlib.colors.TwoSlopeNorm
api/prev_api_changes/api_changes_3.2.0/deprecations.rst:266: WARNING: py:obj reference target not found: TwoSlopeNorm
api/prev_api_changes/api_changes_3.2.0/deprecations.rst:266: WARNING: py:obj reference target not found: TwoSlopeNorm
/home/circleci/project/doc/api/prev_api_changes/api_changes_3.2.0/deprecations.rst:266: WARNING: py:obj reference target not found: TwoSlopeNorm
/home/circleci/project/doc/tutorials/colors/colormapnorms.rst:299: WARNING: py:obj reference target not found: TwoSlopeNorm

efiring
efiring approved these changes Oct 20, 2019
View reviewed changes
Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

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

I have a wording suggestion, and I spotted one of the two sources of CircleCI warnings, but otherwise this looks fine.

doc/api/prev_api_changes/api_changes_3.2.0/deprecations.rst Outdated

``DivergingNorm`` was a misleading name because it did not diverge (in
the sense that it is not its self diverging, but is best used with
diverging norms). It has been renamed to `.TwoSlopeNorm` to better represent
Copy link
Member

Choose a reason for hiding this comment

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

Alternative wording: "...misleading name; although the norm was developed with the idea that it would likely be used with diverging colormaps, the word 'diverging' does not describe or evoke the norm's mapping function. Since that function is monotonic, continuous, and piece-wise linear with two segments, the norm has been renamed to .TwoSlopeNorm."

tutorials/colors/colormapnorms.py Outdated
# DivergingNorm: Different mapping on either side of a center
# -----------------------------------------------------------
# TwoSlopeNorm: Different mapping on either side of a center
# ---------------------------------------------------------
Copy link
Member

Choose a reason for hiding this comment

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

Aha! There is the "too short underline".

Copy link
Member Author

Choose a reason for hiding this comment

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

My first pass at this I spelled it TwoSlopNorm and apparently did not fix up all of the fixes...

@tacaswell tacaswell force-pushed the api_rename_divergingnorm branch from 8e3d081 to 64c2bac Compare October 20, 2019 01:17
@tacaswell
Copy link
Member Author

The docs build clean locally, hopefully circle argrees.

@jklymak jklymak merged commit cfd52eb into matplotlib:master Oct 20, 2019
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 20, 2019
@jklymak
Backport PR matplotlib#15450: API: rename DivergingNorm to TwoSlopeNorm
77df15a
@meeseeksmachine meeseeksmachine mentioned this pull request Oct 20, 2019
Merged
jklymak added a commit that referenced this pull request Oct 21, 2019
@jklymak
Merge pull request #15457 from meeseeksmachine/auto-backport-of-pr-15…
a1f054a 
…450-on-v3.2.x

Backport PR #15450 on branch v3.2.x (API: rename DivergingNorm to TwoSlopeNorm)
@tacaswell tacaswell deleted the api_rename_divergingnorm branch October 22, 2019 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efiring efiring efiring approved these changes

Assignees
No one assigned
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.

DivergingNorm is a misleading name
3 participants
@tacaswell @efiring @jklymak