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

Minor expansion of Cython example #5560

Merged
merged 2 commits into from May 8, 2014

Conversation

eteq
Copy link
Contributor

@eteq eteq commented Apr 8, 2014

This just adds a pure-python version of the Black-Scholes function for comparison with the Cython equivalent. I think this better demonstrates why you might want to use the Cython magic.

@Carreau
Copy link
Member

Carreau commented Apr 19, 2014

I like it. +1

Though I know cython magic is moving out of IPython by @mgaitan here
Cf #3803, so maybe the work should focus there.

@mgaitan
Copy link
Contributor

mgaitan commented Apr 21, 2014

@eteq could you make a PR against my cython branch "ipython" https://github.com/mgaitan/cython/tree/ipython . So, it would be part of the "master" PR cython/cython#286.
Thanks

@eteq
Copy link
Contributor Author

eteq commented Apr 21, 2014

Just added the PR to @mgaitan's branch. Perhaps this should also be merged at least until the rest of those examples are removed?

Perhaps this is already decided, @Carreau, but will this example then be removed from the ipython repo? I only have my perspective to offer, but I certainly would not have learned about this feature from looking at Cython given that I don't expect to find ipython notebooks there, but I do of course expect to hear about killer features like this one in the ipython docs. So it would be good for this to still be present in the ipython examples, even if it's just a link to the Cython repo version.

@Carreau
Copy link
Member

Carreau commented Apr 21, 2014

Perhaps this is already decided, @Carreau, but will this example then be removed from the ipython repo?

I think it will, not that we do not like Cython, but because it have higher chance to become outdated
or irrelevant if/when the cython pakage-magic become out of sync with IPython. Or at least it size might diminish.

I only have my perspective to offer, but I certainly would not have learned about this feature from looking at Cython given that I don't expect to find ipython notebooks there, but I do of course expect to hear about killer features like this one in the ipython docs. So it would be good for this to still be present in the ipython examples, even if it's just a link to the Cython repo version.

I encourage you to have a look at the gallery and we of course welcome contributors to any of the subproject related to IPython. One of those would be to improve nbviewer and create a curated version of notebooks, add search, and so on, and so forth. It would be a much sane ecosystem that stuffing things in IPython that get released every 6-9 month.

That being said I have no object to merge this.

@eteq
Copy link
Contributor Author

eteq commented Apr 21, 2014

I realize it may well get out-of-sync, but my personal perspective is that Cython is crucial to some kinds of high-performance numerical work in Python, and hence easy access to it is a killer feature. The gallery is more of a "cookbook" approach, which makes it much less likely users will stumble upon new features by looking there. By contrast, the IPython examples are something I actually looked through when I discovered them, because I know I'll find new IPython capabilities there that are of practical use for me.

Anyway, it's not a big deal to me personally now that I know about this feature... Just a suggestion for others who might be coming from the same place as I.

mgaitan added a commit to mgaitan/cython that referenced this pull request Apr 21, 2014
Add pure-python comparison to ipython magic example. This improves the cython magic notebook example, as proposed in ipython/ipython#5560
@takluyver
Copy link
Member

I think it's OK to improve our examples so long as they're there - if the better content has been written, there's no point not including it. I'll let @ellisonbg merge this, though, because he asked for a lock on the examples for a while.

@takluyver takluyver added this to the 2.1 milestone May 2, 2014
@takluyver takluyver added the docs label May 2, 2014
@mgaitan
Copy link
Contributor

mgaitan commented May 2, 2014

Would be cool to implement also #2286, right?

@eteq
Copy link
Contributor Author

eteq commented May 4, 2014

Ooh, I didn't know about #2286 - super useful! (Which suggests @mgaitan is right that it should be in the example.) I would prefer to have this merged and get the --anotate part in separately, though, as that's a more substantial change to the example to do it right (and that is probably better done in the Cython repo).

@minrk
Copy link
Member

minrk commented May 8, 2014

@ellisonbg does your example notebook work touch the Cython notebook, or can this be merged without getting in your way?

@ellisonbg
Copy link
Member

I think this can be merged.

minrk added a commit that referenced this pull request May 8, 2014
@minrk minrk merged commit b53effc into ipython:master May 8, 2014
minrk added a commit that referenced this pull request May 8, 2014
This just adds a pure-python version of the Black-Scholes function for comparison with the Cython equivalent.  I think this better demonstrates *why* you might want to use the Cython magic.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
…hon-example

Minor expansion of Cython example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants