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

Merge nbconvert into IPython #3500

Merged
merged 861 commits into from Jul 1, 2013
Merged

Merge nbconvert into IPython #3500

merged 861 commits into from Jul 1, 2013

Conversation

minrk
Copy link
Member

@minrk minrk commented Jul 1, 2013

Supersedes PR #3497. This PR merges the actual repo, and is made from the IPython org so all core devs can finish it up.

This pull request takes the nbconvert package and inserts into the main IPython project as IPython.nbconvert. While nbconvert is not done, we need to get it merged ASAP so continue the work. In addition to the code dump I have made some basic changes:

  • Update imports to begin to develop the official API that people will see when importing various things.
  • Fixed the installation logic to include nbconvert.
  • Created an nbconvert sub-command of ipython so users can do ipython nbconvert.

I will try to get the Travis tests to pass (mostly import issues), but this code does not have tests. These will be added in a future PR that @jdfreder has already started on. One question we should figure out before merging:

  • Should nbconvert be a subcommand or its own command?
  • If it is a subcommand, should we call it nbconvert or the simpler convert?

minrk and others added 30 commits March 17, 2013 10:24
	filters.  Filter is now dependent on env. type.
Added more comments and header comments.
	Instead of the horizantal rule following the target via a
	minipage, the needspace package is used.  This frees the
	need to preprocess the target block using the
	sphinxtransformer.  This allows us to rely on Jinja more,
	as intended by nbconvert.

Fixed some macro names to stay PEP8 consistent.
Removed an unnecessary comment.
Unified the output type blocks into the render_output macro.
Centeralized some code into methods
Added more comments
	Small images were getting streched to larger scales.
@minrk minrk mentioned this pull request Jul 1, 2013
5 tasks
@minrk
Copy link
Member Author

minrk commented Jul 1, 2013

Re-posting my own note from #3497:

Should nbconvert be a subcommand or its own command?

I think it should be a subcommand. I would like to eventually get rid of all top-level commands (ipengine, ipcontroller, etc.), so I don't think we should ever add new top-level scripts. We haven't had an official conversation on this matter, though.

@minrk
Copy link
Member Author

minrk commented Jul 1, 2013

Also: woo!

@ellisonbg
Copy link
Member

Ok I see how you did this, thanks for doing it the right way.

@ellisonbg
Copy link
Member

Looks like you missed by last two commits - the ones that fixed the Travis run.

@minrk
Copy link
Member Author

minrk commented Jul 1, 2013

ok, they weren't there when I merged. Will cherry pick.

@minrk
Copy link
Member Author

minrk commented Jul 1, 2013

Scratch that, they were just after the first comments. Should be in now.

@ellisonbg
Copy link
Member

OK our Travis build is passing again. Only one more thing to figure out: do we want to ipython nbconvert or ipython convert as the subcommand? Other than that, I would like to merge so @jdfreder can continue his work on adding tests in the morning.

@minrk
Copy link
Member Author

minrk commented Jul 1, 2013

I don't have strong feelings on nbconvert vs convert - maybe 51% in favor of just convert?

@fperez
Copy link
Member

fperez commented Jul 1, 2013

I sort of preferred nbconvert for being more explicit, but when I really think about it, it's hard to imagine ipython convert foo.ipynb being in any way confusing. So I'm starting to warm up to plain convert myself...

@ellisonbg
Copy link
Member

I think the main question if if convert alone is ambiguous in any way. I can't think of anything else we would want to "convert" at the command line.

@fperez
Copy link
Member

fperez commented Jul 1, 2013

That's what I was thinking, and honestly from that perspective, I don't see a problem with plain convert.

@fperez
Copy link
Member

fperez commented Jul 1, 2013

Though if we really want to remain future-proof, the nb prefix isn't that terrible either. Honestly I can be persuaded either way, I don't really have a very strong preference.

@ellisonbg
Copy link
Member

I too am just about 50/50...makes it a bit tough to decide. Another reason
to use nbconvert is that is what we have been calling it so far.

On Sun, Jun 30, 2013 at 10:39 PM, Fernando Perez
notifications@github.comwrote:

Though if we really want to remain future-proof, the nb prefix isn't that
terrible either. Honestly I can be persuaded either way, I don't really
have a very strong preference.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3500#issuecomment-20264196
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@jdfreder
Copy link
Member

jdfreder commented Jul 1, 2013

@ellisonbg don't worry about me, I can continue to work on the tests on my machine even though this isn't merged yet. The tests aren't really coupled in a way that would make them break significantly when moved to ipy (just references, easy fix).

@ellisonbg
Copy link
Member

@jdfreder and I talked about the name of the subcommand. Both of us feel that using nbconvert is slightly better as it is consistent with the code base. This consistency is important because users will interact with the code base in config files. Merging - great work everyone!

ellisonbg added a commit that referenced this pull request Jul 1, 2013
Merge nbconvert into IPython
@ellisonbg ellisonbg merged commit fba37df into master Jul 1, 2013
@Carreau
Copy link
Member

Carreau commented Jul 1, 2013

Thanks a lot everyone, I would really like to be able to participate more, but those day I really can't.
Will try to catch up for next hangout.

@damianavila
Copy link
Member

Great! I just landed a few hours ago and it was already merged... great again! Good job!
And thanks for preserving the history of the commits... now, I am an official IPython contributor! ;-)

Cheers.
Damián.

@minrk minrk deleted the nbconvert branch June 20, 2014 18:17
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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