Skip to content

Conversation

@yuecong
Copy link
Contributor

@yuecong yuecong commented Mar 20, 2015

As for "notebook --pylab inline" is not supported any more, update the related documentation for this.

As for "notebook --pylab inline" is not supported any more, update the related documentation for this.
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, it's not supported as of what version -- something old, or recent?
Nit: you can back-tick-quote the %pylab inline bit.
Is it worth explaining what this option does, then? (Could be entirely obvious, just asking.)

@yuecong
Copy link
Contributor Author

yuecong commented Mar 23, 2015

It is IPython 3.0. I believe one of the motivation of IPython notebook is their inline chart drawing functionality. This is the reason why pylab library is specially mentioned.

@srowen
Copy link
Member

srowen commented Mar 23, 2015

But does this then work with ipython 2? I wouldn't want to necessarily 'break' support, even if it's just in an example. Or are two examples called for? Ideally, one example is good, even if it's deprecated in new ipython versions.

@yuecong
Copy link
Contributor Author

yuecong commented Mar 23, 2015

The notebook UI is different between 2.4 and new 3.0. But %pylab inline both work for 2.4 and 3.0 version.
I agree that we do not need to explain how to launch a notebook UI from IPython. Instead, just tell the users they can add %pylab inline into the notobook to use pylab library.
Do I need to update the patch then or the admin will handle with this?

@srowen
Copy link
Member

srowen commented Mar 23, 2015

OK, if this isn't making the example unusable for reasonably recent versions of ipython then it seems OK. Oh, above I was not suggesting that you remove your additional documentation but instead that you explain what the "inline" option was all about? or will it be obvious already?

@yuecong
Copy link
Contributor Author

yuecong commented Mar 23, 2015

Let me clarify my opinions more clearly.
1, change '$ PYSPARK_DRIVER_PYTHON=ipython PYSPARK_DRIVER_PYTHON_OPTS="notebook --pylab inline" ./bin/pyspark' to $ PYSPARK_DRIVER_PYTHON=ipython PYSPARK_DRIVER_PYTHON_OPTS="notebook " ./bin/pyspark'.
For this, we agree with it as for it will not work any more for ipython 3.0
2, Whether it is necessary to methion '%pylab inline' or not.
I think it is necessary, as for this give the users to understand that with ipython notebook, they can visualize their data with pylab, which is different from ipython shell.
3, Whether it needs to add how to launch a notebook from ipython notebook UI.
Originally, I added the explanation on the base of ipython3.0, but as you commented, I find the ipython notebook UI is different between 2.x and 3.x, so I agree we may do not need to explain it in detail to make the guide be suitable for all version of ipythons.

Hope the above can clarify my opinions. :)

@srowen
Copy link
Member

srowen commented Mar 23, 2015

OK, nevermind my question. I think it's clear you know what to do here and it's as you think it should be. I'll leave it open a bit for any other opinions but if it's making the example work for more ipython versions, fine.

@asfgit asfgit closed this in c12312f Mar 24, 2015
asfgit pushed a commit that referenced this pull request Mar 24, 2015
As for "notebook --pylab inline" is not supported any more, update the related documentation for this.

Author: Cong Yue <[email protected]>

Closes #5111 from yuecong/patch-1 and squashes the following commits:

872df76 [Cong Yue] Update the command to use IPython notebook

(cherry picked from commit c12312f)
Signed-off-by: Sean Owen <[email protected]>
@srowen
Copy link
Member

srowen commented Mar 24, 2015

OK I merged this with a couple fixes to the text (e.g. spark -> Spark)

@yuecong
Copy link
Contributor Author

yuecong commented Mar 24, 2015

Thanks.

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.

3 participants