Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Scipy2013 Sprint: Cleaning examples of api example#2181

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
gabraganca wants to merge7 commits intomatplotlib:masterfromgabraganca:api-cleanup
Closed

Scipy2013 Sprint: Cleaning examples of api example#2181

gabraganca wants to merge7 commits intomatplotlib:masterfromgabraganca:api-cleanup

Conversation

gabraganca
Copy link
Contributor

Worked on two examples.

  • Barchart demo. I broke it in two; one with error and other without. Moved it do Statistics folder
  • Fahrenheit and Celsius scale. Cleaned with help of@leouieda and changed to subplots_axes_and_figures folder.

import numpy as np

fig, ax1 = plt.subplots() # ax1 is the Fahrenheit scale
ax2 = ax1.twinx() # ax2 is the Celsius scale
Copy link
Contributor

Choose a reason for hiding this comment

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

Really Minor: Maybe move figure/axes definitions down with the rest of the plotting code (i.e. below the function defs). Also, I think the inline comments should be removed here. (Maybeax1 ->ax_f,ax2 ->ax_c?)

@tonysyu
Copy link
Contributor

Thanks for cleaning up some examples!

I think the fahrenheit/celsius scale example is more or less ready to go. (I had some minor comments, but nothing major.)

The barchart examples may need a bit more thought. The original example showed a few more features than just error bars. I'd be inclined to split the example up intobarchart_demo andbarchart_demo_features, where the latter retains more of the features of the original. In fact,barchart_demo.py inexamples/pylab_examples (notexamples/api) is a pretty clean version of this example.

As suggested in theMEP12 guidelines, you need to check whether the examples you move are referenced in the docs. I believebarchart_demo.py is referenced in thebackend_driver tests. Also, you should consider running a PEP8 checker just to ensure you don't miss anything.

@gabraganca
Copy link
ContributorAuthor

Yeah. Your suggestion seems more reasonable on the farenheit/celsius plot.

So there is already a cleaned version of the barchart? I will check. And also will check about the backend_driver.

@gabraganca
Copy link
ContributorAuthor

@tonysyu, just changed the F/C plot. The other is still pending.

@tacaswell
Copy link
Member

@gabraganca What is the state of this?

@tacaswell
Copy link
Member

Seems to have install issues with py3 and unicode strings.

@gabraganca
Copy link
ContributorAuthor

I had totally forgot about this.

@gabraganca
Copy link
ContributorAuthor

Since it has been a long time that I do not work on this, what would you recommend? I should do a rebase and then let the Travis check again?

@tacaswell
Copy link
Member

It looks like the F/C demo is good, but the barchart demo still needs some thought/merging withpylab version of the demos.

How about pulling the F/C demo changes out into a new PR which we can get merged straight away and, unless you are willing to put more time into cleaning up the barchart demo, dropping that part?

@gabraganca
Copy link
ContributorAuthor

Ok. I will do this now.

Sign up for freeto 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
v1.4.0
Development

Successfully merging this pull request may close these issues.

3 participants
@gabraganca@tonysyu@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp