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

Deprecate support for dash-offset = None. by anntzer · Pull Request #15828 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Deprecate support for dash-offset = None. - #15828

Merged
QuLogic merged 1 commit into
matplotlib:masterfrom
anntzer:dashoffset-none
Mar 10, 2020
ConversationCommits (1)ChecksFiles changed
Merged

Deprecate support for dash-offset = None.#15828
QuLogic merged 1 commit into
matplotlib:masterfrom
anntzer:dashoffset-none

Conversation

anntzer commented Dec 4, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

e.g. plot([1, 2], ls=(None, (4, 4))) which was previously a synonym
for plot([1, 2], ls=(0, (4, 4))) which means "dash-pattern of
4pt-long dashes separated by 4pt spaces, with an offset of 0pt at
start".

Passing None instead of 0 was already not (likely, never) supported for
pdf, ps, or svg (an exception is raised at savefig() time for ps/svg, an
invalid pdf is generated). There isn't much of a point in supporting
None (this also makes e.g. mplcairo more complex because of static
typing in C++ extensions), so just deprecate it.

Goes on top of #15827.
Edit: closes #14996, which is basically the issue described here.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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 force-pushed the dashoffset-none branch 2 times, most recently from 87e9acf to 9300485 Compare December 5, 2019 01:02
tacaswell added this to the v3.3.0 milestone Dec 5, 2019

tacaswell commented Dec 5, 2019

Copy link
Copy Markdown
Member

Could we normalize None as input to 0 instead?

The failures look real, but did not dig into them....

anntzer commented Dec 5, 2019

Copy link
Copy Markdown
Contributor Author

The failures are real, need to look into it.
I think the (small) cleanup to Line2D from not normalizing (allowing to remove a few checks for None) is also nice to have. Again note that this has (likely) never worked for pdf/ps/svg so I doubt there are many people who rely on it...

anntzer mentioned this pull request Jan 21, 2020
Closed
anntzer force-pushed the dashoffset-none branch 2 times, most recently from 838c6fc to 93f43a3 Compare January 26, 2020 17:15

anntzer commented Jan 26, 2020

Copy link
Copy Markdown
Contributor Author

figured out the failure, now fixed, should be good to go.

QuLogic approved these changes Mar 10, 2020
View reviewed changes

QuLogic 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

Rebase and maybe small tweak to a sentence.

Comment thread doc/api/next_api_changes/deprecations.rst Outdated
Deprecate support for dash-offset = None.
eb76378 
e.g. `plot([1, 2], ls=(None, (4, 4)))` which was previously a synonym
for `plot([1, 2], ls=(0, (4, 4)))` which means "dash-pattern of
4pt-long dashes separated by 4pt spaces, with an offset of 0pt at
start".

Passing None instead of 0 was already not (likely, never) supported for
pdf, ps, or svg (an exception is raised at savefig() time for ps/svg, an
invalid pdf is generated).  There isn't much of a point in supporting
None (this also makes e.g. mplcairo more complex because of static
typing in C++ extensions), so just deprecate it.

anntzer commented Mar 10, 2020

Copy link
Copy Markdown
Contributor Author

edited accordingly, and rebased.

QuLogic merged commit 84dda17 into matplotlib:master Mar 10, 2020
anntzer deleted the dashoffset-none branch March 10, 2020 05:18
QuLogic mentioned this pull request Mar 11, 2020
Closed
anntzer mentioned this pull request May 1, 2020
Closed
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

QuLogic QuLogic approved these changes
timhoffm timhoffm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.3.0

Development

Successfully merging this pull request may close these issues.

format error saving eps figure using custom linestyle

4 participants

Back | FazBrowse Home | New Git URL