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

aesthetics pass on AsyncResult.display_outputs #1822

Merged
merged 2 commits into from Jun 1, 2012

Conversation

minrk
Copy link
Member

@minrk minrk commented May 31, 2012

adds a few delimiters when there are rich outputs, and cleans up a few cases of extra trailing whitespace on stdout/err

adds a few delimiters when there are rich outputs, and cleans up a few cases of extra trailing whitespace on stdout/err
@@ -86,6 +86,15 @@
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to include this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it has some additions

Copy link
Member

Choose a reason for hiding this comment

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

Can we name this file parallel_magics.ipynb to match the naming convention of the other example notebooks?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather not, I think forcing lower-case and no spaces on what are essentially GUI documents is terribly ugly.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I buy that argument, can we change the rest of them to this format then?

On Thu, May 31, 2012 at 8:18 PM, Min RK
reply@reply.github.com
wrote:

@@ -86,6 +86,15 @@

I'd rather not, I think forcing lower-case and no spaces on what are essentially GUI documents is terribly ugly.


Reply to this email directly or view it on GitHub:
https://github.com/ipython/ipython/pull/1822/files#r912781

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

Copy link
Member Author

Choose a reason for hiding this comment

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

...grudgingly ;)

Copy link
Member

Choose a reason for hiding this comment

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

On Thu, May 31, 2012 at 8:19 PM, Brian E. Granger
reply@reply.github.com
wrote:

OK, I buy that argument, can we change the rest of them to this format then?

Quick note: let's check that sphinx can handle files with spaces in
them. We'll very soon want to include the notebooks in the built
docs, and I'm not sure sphinx is happy with spaces. In my recent
experiments with Latex I had to rename a notebook that had a space in
it b/c LaTeX barfed all over the spaces.

So before we create notebooks that can't be converted, let's make sure
that the downstream tools will be able to handle them.

Copy link
Member

Choose a reason for hiding this comment

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

On Thu, May 31, 2012 at 8:32 PM, Min RK
reply@reply.github.com
wrote:

...grudgingly ;)

No, I'm happy to go with it: I just want to make sure it won't cause
us headaches later on, based on the recent problems I had precisely
with a similarly named notebook with LaTeX conversion... So if it
turns out that we can make the pipeline for managing them happy with
spaces, I'm happy to go along :)

Copy link
Member Author

Choose a reason for hiding this comment

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

If anything, I would prefer the exporter to handle normalization to a name appropriate for the destination.

Copy link
Member

Choose a reason for hiding this comment

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

I am fine with that.

@fperez
Copy link
Member

fperez commented Jun 1, 2012

Test results for commit 19abacd merged into master
Platform: linux2

  • python2.7: OK (libraries not available: wx.aui)
  • python3.2: OK (libraries not available: cython matplotlib pymongo tornado wx wx.aui)

Not available for testing:

@ellisonbg
Copy link
Member

I ran the example notebook and looked at the output. Test suite passes. I think it looks great and can be merged.

@ellisonbg
Copy link
Member

Opps, hold on I think I have found a problem...

@ellisonbg
Copy link
Member

Yes, the cell in the example notebook that reads %px print >> sys.stderr, "ERROR" gives an error because sys is has not been imported into the engines.

@minrk
Copy link
Member Author

minrk commented Jun 1, 2012

Crap, that's what I get for having a few stdlib imports in my startup dir. Fixed.

@ellisonbg
Copy link
Member

I think this looks good then, if there is nothing else you think needs
to be done, I think this can be merged.

On Thu, May 31, 2012 at 10:27 PM, Min RK
reply@reply.github.com
wrote:

Crap, that's what I get for having a few stdlib imports in my startup dir.  Fixed.


Reply to this email directly or view it on GitHub:
#1822 (comment)

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

@fperez
Copy link
Member

fperez commented Jun 1, 2012

Go from my side too.

@minrk
Copy link
Member Author

minrk commented Jun 1, 2012

Thanks, merging then.

My Python 40k language feature request: add sys, os, re, time stdlib modules to builtins.

minrk added a commit that referenced this pull request Jun 1, 2012
aesthetics pass on AsyncResult.display_outputs

adds a few delimiters when there are rich outputs, and cleans up a few cases of extra trailing whitespace on stdout/err
@minrk minrk merged commit da8a781 into ipython:master Jun 1, 2012
@minrk minrk deleted the asyncaesthetics branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
aesthetics pass on AsyncResult.display_outputs

adds a few delimiters when there are rich outputs, and cleans up a few cases of extra trailing whitespace on stdout/err
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

4 participants