Skip to content

Table.pprint() header ignores stdout redirection - #2878

Merged
embray merged 2 commits into
astropy:masterfrom
embray:issue-2878
Aug 22, 2014
Merged

Table.pprint() header ignores stdout redirection#2878
embray merged 2 commits into
astropy:masterfrom
embray:issue-2878

Conversation

@embray

@embray embray commented Aug 21, 2014

Copy link
Copy Markdown
Member

As found in #2869, if one tries to redirect the output from Table.pprint(), the data get redirected, but the header does not (see below). My guess is that this is because color_print, which is used for the header, does not write to stdout.

In [1]: from astropy.table import Table, Column

In [2]: from contextlib import redirect_stdout

In [3]: t = Table([Column([1., 2.], 'a', unit='m'), Column(['a', 'b'], 'b')])

In [4]: t.pprint()
 a   b 
 m     
--- ---
1.0   a
2.0   b

In [5]: with redirect_stdout(open("a.out", "w")):
    t.pprint()
   ...:     
 a   b 
 m     
--- ---

In [6]: !cat a.out
1.0   a
2.0   b

@embray

embray commented Aug 20, 2014

Copy link
Copy Markdown
Member

ISTM the real issue here is incompatibility between color_print and the redirect_stdout context manager. In particular, because you're in IPython color_print is going through IPython.utils.io.stdout instead of directly through sys.stdout which probably has something to do with it...

@embray embray added this to the v0.4.2 milestone Aug 20, 2014
@embray

embray commented Aug 20, 2014

Copy link
Copy Markdown
Member

In fact that's exactly it. In the context manager sys.stdout is replace by (), while IPython.utils.io.stdout still references the original sys.stdout it was started with.

@mhvk

mhvk commented Aug 21, 2014

Copy link
Copy Markdown
Contributor Author

@embray - ok, looked at astropy.utils.console now and see what you mean. But am also confused: why would color_print not simply write to sys.stdout? After all, ipython seems to just redirect this to its own output scheme, so it should just work. Indeed, if I explicitly do

astropy.utils.console.color_print('abc\n', 'red', file='sys.stdout')

it correctly prints to file with the redirection, and correctly prints color before and after.

Would using sys.stdout as the default for color_print break on non-linux systems or so?

@embray

embray commented Aug 21, 2014

Copy link
Copy Markdown
Member

It specifically has to do with using IPython's IOStream objects which is the easiest way to get that feature to work properly in IPython on Windows.

I'll have to experiment, but I think it should be possible to make our own IPython.utils.io.IOStream around whatever sys.stdout currently happens to be, which should suffice.

@embray

embray commented Aug 21, 2014

Copy link
Copy Markdown
Member

Incidentally, this appears to be broken now in IPython 2.0 where I get:

AttributeError: IOStream instance has no attribute 'name'

so definitely needs to be reworked :(

@embray
This reworks how the correct 'stdout' stream to use by default
882c49a 
for functions like color_print, ProgressBar, etc. is obtained.

Usually we just want to use sys.stdout, but on Windows that does
not work for color.  Previously there were some hacks to instead
use IPython's stdio stream on Windows since it actually wraps
pyreadline and *does* handle colors.  However, by *always* using
IPython's stdout, functions that assume we're using sys.stdout
do not actually write properly to sys.stdout when sys.stdout is
*changed* (e.g. replaced with a log file).

This instead uses sys.stdout by default in *most* cases.  But when
sys.stdout is set to a TTY (the only context where we probably want
to keep colors) *then* it switches over to using IPython's stdout.

An alternative might be to create a wrapper object that handles
this more transparently, but for now this appears to work.
@embray

embray commented Aug 21, 2014

Copy link
Copy Markdown
Member

Attached a reworking of how the correct stdout is to be determined. See the commit message for more details.

Note: There's not much point in trying to cook up a regression test for this since this is only really an issue in IPython anyways.

@mhvk

mhvk commented Aug 22, 2014

Copy link
Copy Markdown
Contributor Author

I guess I am still confused why on windows it is not fine to just write to sys.stdout in the first place (doesn't ipython redirect it anyway; seems like one is working around an upstream bug). But I am also very happy not to have to think about windows, and this does seem all OK otherwise!

@embray

embray commented Aug 22, 2014

Copy link
Copy Markdown
Member

No, IPython doesn't redirect sys.stdout. It uses its own standard I/O wrappers for print its own stuff (prompts and such). On Windows it uses pyreadline to handle color support, so it wraps pyreadline for its stdout purposes. Doesn't do anything that otherwise affects the standard sys.stdout.

embray added a commit that referenced this pull request Aug 22, 2014
@embray
Merge pull request #2878 from embray/issue-2878
7ce7226 
Table.pprint() header ignores stdout redirection
@embray
embray merged commit 7ce7226 into astropy:master Aug 22, 2014
@embray
embray deleted the issue-2878 branch August 22, 2014 19:42
embray added a commit that referenced this pull request Sep 18, 2014
@embray
Merge pull request #2878 from embray/issue-2878
61b8167 
Table.pprint() header ignores stdout redirection
@neutrinoceros neutrinoceros mentioned this pull request Feb 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Bug utils

Projects

None yet

Milestone

v0.4.2

Development

Successfully merging this pull request may close these issues.

2 participants

@embray @mhvk