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.

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

Scipy2013 Sprint: Cleaning examples of api example #2181

Closed
wants to merge 7 commits into from
Closed

Scipy2013 Sprint: Cleaning examples of api example #2181

wants to merge 7 commits into from

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. (Maybe ax1 -> ax_f, ax2 -> ax_c?)

@tonysyu
Copy link
Contributor

tonysyu commented Jul 3, 2013

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 into barchart_demo and barchart_demo_features, where the latter retains more of the features of the original. In fact, barchart_demo.py in examples/pylab_examples (not examples/api) is a pretty clean version of this example.

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

@gabraganca
Copy link
Contributor Author

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
Contributor Author

@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
Contributor Author

I had totally forgot about this.

@gabraganca
Copy link
Contributor Author

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 with pylab 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
Contributor Author

Ok. I will do this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants