Skip to content

API: rename DivergingNorm to TwoSlopeNorm#15450

Merged
jklymak merged 2 commits intomatplotlib:masterfrom
tacaswell:api_rename_divergingnorm
Oct 20, 2019
Merged

API: rename DivergingNorm to TwoSlopeNorm#15450
jklymak merged 2 commits intomatplotlib:masterfrom
tacaswell:api_rename_divergingnorm

Conversation

@tacaswell
Copy link
Copy Markdown
Member

@tacaswell tacaswell commented Oct 19, 2019

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

Comment thread 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
Copy Markdown
Member

@efiring efiring Oct 20, 2019

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

Comment thread 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
Copy Markdown
Member

@efiring efiring Oct 20, 2019

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
Copy Markdown
Member Author

@tacaswell tacaswell Oct 20, 2019

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
Copy Markdown
Member Author

tacaswell commented Oct 20, 2019

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