X Tutup
Skip to content

Cleanup for drawstyles.#6564

Merged
tacaswell merged 1 commit intomatplotlib:masterfrom
anntzer:cleanup-drawstyles
Jun 19, 2016
Merged

Cleanup for drawstyles.#6564
tacaswell merged 1 commit intomatplotlib:masterfrom
anntzer:cleanup-drawstyles

Conversation

@anntzer
Copy link
Copy Markdown
Contributor

@anntzer anntzer commented Jun 9, 2016

  1. Remove unused entries from STEP_LOOKUP_MAP (plt.step already
    normalizes drawstyles to step-* format).
  2. Clarify the error for invalid drawstyles.
  3. Fix handling of step-* drawstyles by the Qt options editor.

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Jun 10, 2016

https://travis-ci.org/matplotlib/matplotlib/jobs/136321898#L1084

Apparently that feature got in with out enough tests.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jun 10, 2016
@anntzer
Cleanup for drawstyles.
b45fdde 
1. Remove unused entries from `STEP_LOOKUP_MAP` (`plt.step` already
normalizes drawstyles to `steps-*` format, make `fill_between` and
`fill_betweenx` do the same).

2. Clarify the error for invalid drawstyles.

3. Fix handling of `steps-*` drawstyles by the Qt options editor.
@anntzer anntzer force-pushed the cleanup-drawstyles branch from ba33811 to b45fdde Compare June 10, 2016 06:25
@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Jun 10, 2016

Fixed. I chose to normalize the input of fill_between to steps-*.

@tacaswell tacaswell merged commit c489bff into matplotlib:master Jun 19, 2016
@anntzer anntzer deleted the cleanup-drawstyles branch June 20, 2016 01:02
@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Jul 27, 2016

Actually, can we backport this to 2.0? Up to 1.5.1, opening the figure options editor after making a plot of drawstyle "steps-post" (for example) would print an error and mangle the drawstyle, which is bad, but still much better than the situation in 2.0b3 where an exception in thrown in the event loop (... and by default, PyQt5 aborts the program in such a case).

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Jul 28, 2016

Does the whole PR need to go back, or just the changes to figureoptions?

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Jul 28, 2016

I think the latter should be enough.

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Sep 12, 2016

@tacaswell should I open a new PR with just the changes to figureoptions? Will that lead to merge conflicts when merging 2.0 back into master (if we ever do that?).
TBH I think the PR is small enough that splitting it is a bit overkill but either way works for me.
(Again, note that the change to figureoptions removes a possibility for a fatal exception in the Qt5 backend.)

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Sep 12, 2016

Can you do a PR with just the figureoptions change?

@anntzer anntzer mentioned this pull request Sep 12, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.1

Development

Successfully merging this pull request may close these issues.

3 participants

@anntzer @tacaswell @mdboom
X Tutup