FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

API: rename DivergingNorm to TwoSlopeNorm by tacaswell · Pull Request #15450 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

API: rename DivergingNorm to TwoSlopeNorm - #15450

Merged
jklymak merged 2 commits into
matplotlib:masterfrom
tacaswell:api_rename_divergingnorm
Oct 20, 2019
ConversationCommits (2)ChecksFiles changed
Merged

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

Conversation

tacaswell commented Oct 19, 2019

Copy link
Copy Markdown
Member

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

efiring commented Oct 20, 2019

Copy link
Copy Markdown
Member

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 approved these changes Oct 20, 2019
View reviewed changes

efiring left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

efiring Oct 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

efiring Oct 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Aha! There is the "too short underline".

tacaswell Oct 20, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

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

tacaswell commented Oct 20, 2019

Copy link
Copy Markdown
Member Author

The docs build clean locally, hopefully circle argrees.

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
Backport PR matplotlib#15450: API: rename DivergingNorm to TwoSlopeNorm
77df15a
meeseeksmachine mentioned this pull request Oct 20, 2019
Merged
jklymak added a commit that referenced this pull request Oct 21, 2019
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 deleted the api_rename_divergingnorm branch October 22, 2019 21:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Back | FazBrowse Home | New Git URL