Skip to content
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

Use drawstyle instead of linestyle in plot.step. #3274

Merged
merged 1 commit into from
Mar 26, 2020
Merged

Use drawstyle instead of linestyle in plot.step. #3274

merged 1 commit into from
Mar 26, 2020

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Aug 31, 2019 •
edited
Loading

Mixing the two is deprecated in Matplotlib 3.1, and breaks the doc build
if warnings are set to errors (which they are in new IPython sphinx
extensions.)

  • See Warnings in the test suite #3266
  • Tests added
  • Passes black . && mypy . && flake8
  • Fully documented, including whats-new.rst for all changes and api.rst for new API

@max-sixty
Copy link
Collaborator

Thanks @QuLogic !

If you want to add a whatsnew and give yourself credit, please do.
Given it silences a warning, I don't think we necessarily need a test; if you have an easy one to add then feel free.

@max-sixty max-sixty mentioned this pull request Aug 31, 2019
Merged
2 tasks
@QuLogic QuLogic mentioned this pull request Aug 31, 2019
Closed
@dcherian
Copy link
Contributor

dcherian commented Sep 2, 2019

Hmmm... this is backwards-incompatible so we'll have to be careful.

Mixing the two is deprecated in Matplotlib 3.1

What does this mean? I couldn't find anything in the release notes...

@QuLogic
Copy link
Contributor Author

QuLogic commented Sep 26, 2019

What does this mean? I couldn't find anything in the release notes...

https://matplotlib.org/api/api_changes.html#passing-a-line2d-s-drawstyle-together-with-the-linestyle-is-deprecated

@dcherian
Copy link
Contributor

Looks like our min matplotlib is now 3.1.0. @QuLogic can you add a test and whats-new entry please?

@max-sixty
Copy link
Collaborator

@QuLogic this is so close! How would you feel about a test & whatsnew entry? Then we can merge.

@dcherian dcherian mentioned this pull request Jan 17, 2020
Merged
4 tasks
@QuLogic
Copy link
Contributor Author

QuLogic commented Mar 7, 2020

Sorry I lost track of this; I will rebase soon and update.

@QuLogic QuLogic force-pushed the fix-drawstyle branch 2 times, most recently from b3867c4 to 0e1d26e Compare March 25, 2020 05:00
@QuLogic
Copy link
Contributor Author

QuLogic commented Mar 25, 2020

Done.

@mathause
Copy link
Collaborator

Does this need a deprecation message or is this handled by matplotlib?

max-sixty
max-sixty approved these changes Mar 25, 2020
View reviewed changes
doc/whats-new.rst Outdated Show resolved Hide resolved
doc/whats-new.rst Outdated Show resolved Hide resolved
doc/whats-new.rst Outdated Show resolved Hide resolved
dcherian
dcherian reviewed Mar 25, 2020
View reviewed changes
xarray/plot/plot.py
if ls is not None:
if linestyle is None:
linestyle = ls
if ds is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

raise an error if linestyle or ls is in kwargs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

linestyle and ls are still valid things to pass; you just can't mix draw styles into the string.

@QuLogic
Use drawstyle instead of linestyle in plot.step.
fb9adcb 
Mixing the two is deprecated in Matplotlib 3.1, and breaks the doc build
if warnings are set to errors (which they are in new IPython sphinx
extensions.)
@pep8speaks
Copy link

Hello @QuLogic! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 72:3: E111 indentation is not a multiple of four
Line 85:3: E111 indentation is not a multiple of four
Line 89:3: E111 indentation is not a multiple of four
Line 92:3: E111 indentation is not a multiple of four
Line 101:3: E111 indentation is not a multiple of four
Line 110:3: E111 indentation is not a multiple of four
Line 112:3: E111 indentation is not a multiple of four
Line 115:3: E111 indentation is not a multiple of four
Line 118:3: E111 indentation is not a multiple of four
Line 121:7: E111 indentation is not a multiple of four
Line 124:3: E111 indentation is not a multiple of four
Line 127:7: E114 indentation is not a multiple of four (comment)
Line 128:7: E114 indentation is not a multiple of four (comment)
Line 129:7: E111 indentation is not a multiple of four
Line 132:3: E111 indentation is not a multiple of four
Line 137:3: E111 indentation is not a multiple of four
Line 146:3: E111 indentation is not a multiple of four
Line 148:7: E111 indentation is not a multiple of four
Line 150:7: E111 indentation is not a multiple of four
Line 154:3: E111 indentation is not a multiple of four
Line 162:3: E111 indentation is not a multiple of four
Line 164:7: E111 indentation is not a multiple of four
Line 183:3: E111 indentation is not a multiple of four
Line 208:3: E111 indentation is not a multiple of four
Line 211:3: E111 indentation is not a multiple of four
Line 212:3: E111 indentation is not a multiple of four
Line 218:3: E111 indentation is not a multiple of four
Line 223:3: E111 indentation is not a multiple of four
Line 226:3: E114 indentation is not a multiple of four (comment)
Line 227:3: E114 indentation is not a multiple of four (comment)
Line 228:3: E114 indentation is not a multiple of four (comment)
Line 229:3: E114 indentation is not a multiple of four (comment)
Line 230:3: E114 indentation is not a multiple of four (comment)
Line 231:3: E111 indentation is not a multiple of four
Line 243:3: E111 indentation is not a multiple of four
Line 254:3: E111 indentation is not a multiple of four
Line 255:3: E114 indentation is not a multiple of four (comment)
Line 256:3: E114 indentation is not a multiple of four (comment)
Line 257:3: E114 indentation is not a multiple of four (comment)
Line 258:3: E114 indentation is not a multiple of four (comment)
Line 259:3: E114 indentation is not a multiple of four (comment)
Line 260:3: E111 indentation is not a multiple of four
Line 262:3: E111 indentation is not a multiple of four
Line 265:7: E114 indentation is not a multiple of four (comment)
Line 266:7: E111 indentation is not a multiple of four
Line 267:7: E111 indentation is not a multiple of four
Line 269:7: E111 indentation is not a multiple of four
Line 298:3: E111 indentation is not a multiple of four
Line 299:3: E111 indentation is not a multiple of four
Line 301:3: E111 indentation is not a multiple of four
Line 302:3: E111 indentation is not a multiple of four
Line 303:3: E111 indentation is not a multiple of four
Line 305:3: E114 indentation is not a multiple of four (comment)
Line 306:3: E111 indentation is not a multiple of four
Line 307:3: E111 indentation is not a multiple of four
Line 310:3: E111 indentation is not a multiple of four
Line 317:3: E114 indentation is not a multiple of four (comment)
Line 318:3: E114 indentation is not a multiple of four (comment)
Line 319:3: E111 indentation is not a multiple of four
Line 323:3: E114 indentation is not a multiple of four (comment)
Line 324:3: E111 indentation is not a multiple of four
Line 325:3: E111 indentation is not a multiple of four
Line 328:3: E111 indentation is not a multiple of four
Line 332:3: E111 indentation is not a multiple of four
Line 334:3: E111 indentation is not a multiple of four
Line 338:3: E111 indentation is not a multiple of four
Line 340:3: E111 indentation is not a multiple of four
Line 341:24: W504 line break after binary operator
Line 342:5: E129 visually indented line with same indent as next logical line
Line 343:7: E111 indentation is not a multiple of four
Line 347:3: E111 indentation is not a multiple of four
Line 350:3: E111 indentation is not a multiple of four
Line 355:5: E125 continuation line with same indent as next logical line
Line 356:3: E111 indentation is not a multiple of four
Line 372:3: E114 indentation is not a multiple of four (comment)
Line 373:3: E111 indentation is not a multiple of four
Line 377:3: E111 indentation is not a multiple of four
Line 380:3: E114 indentation is not a multiple of four (comment)
Line 381:3: E111 indentation is not a multiple of four
Line 383:7: E111 indentation is not a multiple of four
Line 384:7: E111 indentation is not a multiple of four
Line 387:7: E111 indentation is not a multiple of four
Line 391:3: E114 indentation is not a multiple of four (comment)
Line 392:3: E114 indentation is not a multiple of four (comment)
Line 393:3: E111 indentation is not a multiple of four
Line 400:3: E111 indentation is not a multiple of four
Line 400:57: W504 line break after binary operator
Line 401:7: E129 visually indented line with same indent as next logical line
Line 405:17: E721 do not compare types, use 'isinstance()'
Line 406:7: E111 indentation is not a multiple of four
Line 407:7: E111 indentation is not a multiple of four
Line 409:3: E111 indentation is not a multiple of four
Line 410:3: E111 indentation is not a multiple of four
Line 428:7: E111 indentation is not a multiple of four
Line 430:7: E111 indentation is not a multiple of four
Line 434:7: E111 indentation is not a multiple of four
Line 435:7: E111 indentation is not a multiple of four
Line 438:11: E125 continuation line with same indent as next logical line
Line 443:3: E111 indentation is not a multiple of four
Line 448:7: E111 indentation is not a multiple of four
Line 452:7: E111 indentation is not a multiple of four
Line 453:7: E111 indentation is not a multiple of four
Line 455:3: E111 indentation is not a multiple of four
Line 456:3: E111 indentation is not a multiple of four
Line 460:3: E111 indentation is not a multiple of four
Line 462:3: E111 indentation is not a multiple of four
Line 468:3: E111 indentation is not a multiple of four
Line 470:3: E111 indentation is not a multiple of four
Line 477:3: E111 indentation is not a multiple of four
Line 494:3: E111 indentation is not a multiple of four
Line 496:3: E111 indentation is not a multiple of four
Line 498:3: E111 indentation is not a multiple of four
Line 500:3: E111 indentation is not a multiple of four
Line 502:7: E111 indentation is not a multiple of four
Line 505:3: E111 indentation is not a multiple of four
Line 509:3: E111 indentation is not a multiple of four
Line 511:3: E111 indentation is not a multiple of four
Line 520:3: E111 indentation is not a multiple of four
Line 521:3: E111 indentation is not a multiple of four
Line 522:3: E111 indentation is not a multiple of four
Line 539:3: E111 indentation is not a multiple of four
Line 543:3: E111 indentation is not a multiple of four
Line 545:3: E111 indentation is not a multiple of four
Line 546:3: E111 indentation is not a multiple of four
Line 547:3: E111 indentation is not a multiple of four
Line 548:3: E114 indentation is not a multiple of four (comment)
Line 550:3: E114 indentation is not a multiple of four (comment)
Line 551:3: E114 indentation is not a multiple of four (comment)
Line 552:3: E114 indentation is not a multiple of four (comment)
Line 553:3: E114 indentation is not a multiple of four (comment)
Line 554:3: E114 indentation is not a multiple of four (comment)
Line 555:3: E111 indentation is not a multiple of four
Line 558:7: E111 indentation is not a multiple of four
Line 565:3: E114 indentation is not a multiple of four (comment)
Line 566:3: E111 indentation is not a multiple of four
Line 568:3: E111 indentation is not a multiple of four
Line 569:3: E111 indentation is not a multiple of four
Line 570:3: E111 indentation is not a multiple of four
Line 572:3: E111 indentation is not a multiple of four
Line 574:3: E111 indentation is not a multiple of four
Line 576:3: E111 indentation is not a multiple of four
Line 579:3: E111 indentation is not a multiple of four
Line 584:3: E111 indentation is not a multiple of four
Line 585:3: E111 indentation is not a multiple of four
Line 587:3: E111 indentation is not a multiple of four
Line 594:3: E111 indentation is not a multiple of four
Line 596:3: E111 indentation is not a multiple of four
Line 599:3: E111 indentation is not a multiple of four
Line 603:3: E111 indentation is not a multiple of four
Line 615:3: E114 indentation is not a multiple of four (comment)
Line 616:3: E111 indentation is not a multiple of four
Line 618:3: E111 indentation is not a multiple of four
Line 619:3: E111 indentation is not a multiple of four
Line 620:3: E111 indentation is not a multiple of four
Line 621:3: E111 indentation is not a multiple of four
Line 622:3: E111 indentation is not a multiple of four
Line 623:3: E111 indentation is not a multiple of four
Line 625:3: E114 indentation is not a multiple of four (comment)
Line 626:3: E114 indentation is not a multiple of four (comment)
Line 627:3: E114 indentation is not a multiple of four (comment)
Line 628:3: E111 indentation is not a multiple of four
Line 629:3: E111 indentation is not a multiple of four
Line 630:3: E111 indentation is not a multiple of four
Line 633:3: E111 indentation is not a multiple of four
Line 634:3: E111 indentation is not a multiple of four
Line 635:3: E111 indentation is not a multiple of four
Line 636:3: E111 indentation is not a multiple of four
Line 637:3: E111 indentation is not a multiple of four

@QuLogic
Copy link
Contributor Author

QuLogic commented Mar 25, 2020

@pep8speaks seems a little broken... Line 328 is an empty line.

@max-sixty
Copy link
Collaborator

Yes agree! Let's merge on green

@max-sixty max-sixty merged commit 6378a71 into pydata:master Mar 26, 2020
@max-sixty
Copy link
Collaborator

Thanks @QuLogic !

@dcherian
Copy link
Contributor

Got it. Thanks @QuLogic

@QuLogic QuLogic deleted the fix-drawstyle branch March 26, 2020 00:57
dcherian added a commit to dcherian/xarray that referenced this pull request Mar 28, 2020
@dcherian
Merge remote-tracking branch 'upstream/master' into map-blocks-schema
4a355e6 
* upstream/master: (54 commits)
  Limit repr of arrays containing long strings (pydata#3900)
  expose a few zarr backend functions as semi-public api (pydata#3897)
  Use drawstyle instead of linestyle in plot.step. (pydata#3274)
  Implementation of polyfit and polyval (pydata#3733)
  misplaced quote in whatsnew (pydata#3889)
  Rename ordered_dict_intersection -> compat_dict_intersection (pydata#3887)
  Control attrs of result in `merge()`, `concat()`, `combine_by_coords()` and `combine_nested()` (pydata#3877)
  xfail test_uamiv_format_write (pydata#3885)
  Use `fixes` in PR template (pydata#3886)
  Tweaks to "how_to_release" (pydata#3882)
  whatsnew section for 0.16.0
  Release v0.15.1
  whatsnew for 0.15.1 (pydata#3879)
  update panel documentation (pydata#3880)
  reword the whats-new entry for unit support (pydata#3878)
  Raise error when assigning to IndexVariable.values & IndexVariable.data (pydata#3862)
  Re-enable tests xfailed in pydata#3808 and fix new CFTimeIndex failures due to upstream changes (pydata#3874)
  add spacing in the versions section of the issue report (pydata#3876)
  map_blocks: allow user function to add new unindexed dimension. (pydata#3817)
  Delete associated indexes when deleting coordinate variables. (pydata#3840)
  ...
dcherian added a commit to MeraX/xarray that referenced this pull request Mar 29, 2020
@dcherian
Merge remote-tracking branch 'upstream/master' into patch-1
068aac7 
* upstream/master: (75 commits)
  Implement idxmax and idxmin functions (pydata#3871)
  Update pre-commit-config.yaml (pydata#3911)
  Revert "Use `fixes` in PR template (pydata#3886)" (pydata#3912)
  update the docstring of diff (pydata#3909)
  Un-xfail test_dayofyear_after_cftime_range (pydata#3907)
  Limit repr of arrays containing long strings (pydata#3900)
  expose a few zarr backend functions as semi-public api (pydata#3897)
  Use drawstyle instead of linestyle in plot.step. (pydata#3274)
  Implementation of polyfit and polyval (pydata#3733)
  misplaced quote in whatsnew (pydata#3889)
  Rename ordered_dict_intersection -> compat_dict_intersection (pydata#3887)
  Control attrs of result in `merge()`, `concat()`, `combine_by_coords()` and `combine_nested()` (pydata#3877)
  xfail test_uamiv_format_write (pydata#3885)
  Use `fixes` in PR template (pydata#3886)
  Tweaks to "how_to_release" (pydata#3882)
  whatsnew section for 0.16.0
  Release v0.15.1
  whatsnew for 0.15.1 (pydata#3879)
  update panel documentation (pydata#3880)
  reword the whats-new entry for unit support (pydata#3878)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcherian dcherian dcherian left review comments

@max-sixty max-sixty max-sixty approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@QuLogic @max-sixty @dcherian @mathause @pep8speaks