Stdout redirection #1009

henryiii · 2017-08-18T03:00:46Z

This is the std::cout redirection discussed in #1005. The new docs:

Capturing standard output from ostream

Often, a library will use the streams std::cout and std::cerr to
print, but this does not play well with Python's standard sys.stdout
and sys.stderr redirection. Replacing a library's printing with
py::print may not be feasible. This can be fixed using a guard
around the library function that redirects output to the corresponding
python streams:

// At beginning of file
#include <pybind11/iostream.h>

...

// Add a scoped redirect for your noisy code
m.def("noisy_func", []() {
    py::scoped_ostream_redirect stream(
        std::cout,                               // std::ostream&
        py::module::import("sys").attr("stdout") // Python output
    );
    call_noisy_func();
});

This method respects flushes on the output streams, and will flush if
needed when the scoped guard is destroyed. This allows the output to be
redirected in real time, such as to a Jupyter notebook. The two
arguments, the C++ stream and the Python output, are optional, and
default to standard output if not given. An extra type,
py::scoped_estream_redirect, is identical execpt for defaulting to the
error stream; this can be useful with py::call_guard, which allows
multiple items, but uses the default constuctor:

// Alternative: Call single function using call guard
m.def("noisy_func", &call_noisy_function,
      py::call_guard<py::scoped_ostream_redirect,
                     py::scoped_estream_redirect>());

The redirection can also be done in python with the addition of a
context manager, using the py::add_ostream_redirect() function:

py::add_ostream_redirect(m, "ostream_redirect");

The name in Python defaults to ostream_redirect if no name is passed.
This creates the following context manager in Python:

with ostream_redirect(stdout=True, stderr=True):
    noisy_function()

The added context manager defaults to redirecting both streams, though
you can use the keyword arguments to disable one of the streams if
needed.

Closes #1005.

henryiii · 2017-08-18T04:04:53Z

Odd, you don't seem to be able to del a py::container in Pypy, it calls the destructor twice (segfault).

aldanor · 2017-08-18T11:31:44Z

My subjective 2c: this is somewhat half-baked since it doesn't work with C-level output.

In fact, this is possible to implement completely outside of pybind11 -- e.g. I've done something similar in ipybind that works via os.dup2 and redirects all kinds of streams.

henryiii · 2017-08-18T13:05:59Z

As long as you are working with C++, this works, and it does the redirection directly in the C++ code. Since libraries usually don't change IO, this either works if cout/cerr is used, or it doesn't if printf and raw file descriptors are used for a given library. This is much simpler and completely portable, and can be added in C++ instead of wrapping in Python.

This could be extended eventually to provide a FD version; it would need a series of platform checks, but that code (even on Python) has a series of checks as well. Like the Python version, it would not be fully standard, relying on non-standard extensions like __stdoutp. Libraries that do this, like PyROOT, tend to behave weirdly.

henryiii · 2017-08-18T13:06:36Z

Just my 2c as well, and I'm biased. 😉

Also, is ipybind listed somewhere officially? It seems like a very useful tool!

dean0x7d

The C-level redirects were pretty flakey when we were using them in pytest (before py::print). The platform-specific implementations also make them complicated.

The C++ specific solution in this PR seems pretty nice to me: simple and portable. Looks good overall, but I left some comments which should be addressed.

Some other things:

I'm not sure if this should be a new header or just added to pybind11/stl.h. Does anyone else have any opinions on this? pybind11/stl.h does already include and define a custom stream operator<< for py::object which is kind of related. Never mind, as @jagerman points out below, stl.h is for type casters.
Maybe add a note in the docs that only std streams are redirected, but not C output functions.
Some of the output capture is leaking into the test log. Ideally, that shouldn't be there.

dean0x7d · 2017-08-19T17:09:03Z

docs/advanced/pycpp/utilities.rst

@@ -23,6 +23,39 @@ expected in Python:

 .. _eval:

+Capturing ``std::cout`` and ``std::cerr`` usage
+===============================================


The .. _eval: tag has ended up in the wrong place. It should point to the section below.

dean0x7d · 2017-08-19T17:31:20Z

include/pybind11/iostream.h

+
+class pythonbuf : public std::streambuf {
+private:
+    typedef std::streambuf::traits_type traits_type;


typedef -> using.

dean0x7d · 2017-08-19T17:45:17Z

include/pybind11/iostream.h

+
+    int sync() {
+        if (this->pbase() != this->pptr()) {
+            std::string line(this->pbase(), this->pptr());


The write() call is going to do an automatic std::string -> py::str conversion. But you could also construct py::str directly using its (char *, size_t) constructor.

dean0x7d · 2017-08-19T17:46:54Z

include/pybind11/iostream.h

+
+            auto write = pyostream.attr("write");
+            write(line);
+            pyostream.attr("flush")();


Consider making write and flush member variables instead of pyostream.

dean0x7d · 2017-08-19T17:50:48Z

include/pybind11/iostream.h

+    }
+};
+
+class opythonstream : private virtual pythonbuf, public std::ostream {


Can't pythonbuf just be a member variable? To avoid multiple inheritance.

Not trivially, since it will be constructed in the wrong order, as the final object is required to construct the class it would be a member of. Let me know if you have any ideas other than manually building objects first and then passing around smart pointers or similar.

dean0x7d · 2017-08-19T17:57:53Z

tests/CMakeLists.txt

@@ -40,6 +40,8 @@ set(PYBIND11_TEST_FILES
  test_eval.cpp
  test_exceptions.cpp
  test_factory_constructors.cpp
+  test_globaliostream.cpp
+  test_iostream.cpp


Regardless of the header question above, I think it would be best to merge these into test_stl.cpp since there's not a lot of new stream tests. Adding complete new translation units really bloats the compile time of the test suite.

Also, merge the individual test functions into test_globaliostream() and test_iostream() when moved into test_stl.py.

Merged globaliostream and iostream, can easily merge iostream to stl, for example, let me know if it's still a good idea, and if the py file should be merged too. I like having separate files, since there's not much shared between them, but can change it easily.

dean0x7d · 2017-08-19T18:07:59Z

tests/test_globaliostream.cpp

+TEST_SUBMODULE(globaliostream, m) {
+    // test_evals
+    m.def("redirect_output", []() {
+            return py::capsule(new py::scoped_output_redirect(


I see that this is going for an RAII-style guard, but those should really be implemented using context managers in Python. Something like the following (not actually tested):

struct RedirectOutput { std::unique_ptr<py:: scoped_output_redirect> redirect; } py::class_<RedirectOutput>(m, "redirect_output") .def("__enter__", [](RedirectOutput &self) { self.redirect.reset(new py::scoped_output_redirect(...)) }) .def("__exit__", [](RedirectOutput &self, py::args) { self.redirect.reset(); });

Note: it's convention for Python context managers to have lowercase names.

with m.redirect_output(): # redirected here with capture: # can be nested

Redesigned, and made available in the iostream.h header. I've tried to tweak the names a bit too to be clearer.

henryiii · 2017-08-20T01:02:17Z

Fixed the easier comments, will work on the rest of it tomorrow. Thanks for the review!

I could put it in the detail folder then include in stl.h.

henryiii · 2017-08-20T21:41:56Z

Remaining tasks:

Try removing multiple inheritance (?)
Add note about C output to docs
Update docs for new redirect
Try idea about removing non-captured output
Move tests into stl (last, since it's easier to develop by itself) (question posed below)

jagerman · 2017-08-20T22:05:25Z

I'm not sure if this should be a new header or just added to pybind11/stl.h. Does anyone else have any opinions on this?

I vote, at least weakly, in favour of a new header. stl.h's current self-professed (i.e. in the file header) purpose is: Transparent conversion for STL data types. This feature, on the other hand, doesn't really fit with that. While it lets you manipulate the stl output interface, it isn't exactly "conversion" nor is it transparent. It seems different enough to me to warrant its own header.

dean0x7d · 2017-08-20T22:14:08Z

stl.h's current self-professed (i.e. in the file header) purpose is: Transparent conversion for STL data types

Ah, indeed. And with the proposed header reorg from #708 it would become pybind11/cast/stl.h. The cast part definitely doesn't fit here. Then it would be best to keep it iostream.h as is.

henryiii · 2017-08-21T06:10:17Z

That's most of the changes, plus a bit of renaming/redesign based on the comments. The remaining questions: multiple inheritance doesn't seem clean to avoid, due to construction order. The class is otherwise very simple, doing almost nothing past the multiple inheritance part.

The second question is if the test files should be combined. I like keeping the python files separate, but I can combine the C++ files (I have a local change stashed that I can push up to do that if that's still agreed upon). The code in the files is mostly orthogonal.

jagerman · 2017-08-21T15:04:06Z

multiple inheritance doesn't seem clean to avoid, due to construction order.

Would something like the following work?

class opythonstream : public std::ostream {
public:
    opythonstream(object pyostream) : std::ostream(new pythonbuf(pyostream)) { }
    virtual ~opythonstream() { delete rdbuf(nullptr); }
};

dean0x7d

Looks pretty good to me. The separate test file is fine. Just one more thing: It should be possible to make a buffer simplification -- see the comment below.

dean0x7d · 2017-08-21T20:05:43Z

include/pybind11/iostream.h

@@ -0,0 +1,188 @@
+#pragma once


Add a license notice like in test_iostream.cpp and the other headers.

Copied from there.

dean0x7d · 2017-08-21T20:30:04Z

include/pybind11/iostream.h

+            std::ostream& costream = std::cout,
+            object pyostream = module::import("sys").attr("stdout") )
+        : costream(costream), buffer(pyostream) {
+        old = costream.rdbuf(buffer.rdbuf());


Seems like the opythonstream class isn't even needed. The buffer could just be:

detail::pythonbuf buffer;

And then:

old = costream.rdbuf(&buffer);

That makes it simpler and cleaner! Done.

dean0x7d · 2017-08-21T22:47:28Z

Oh, cool, I just realized that this can be combined with call guard:

m.def("noisy_func", &noisy_func, py::call_guard<py::scoped_ostream_redirect>());

henryiii · 2017-08-21T23:12:18Z

You are right, I'm adding that to the tests and docs 👍

henryiii · 2017-08-21T23:55:50Z

Added tests to verify flushing worked, and auto-flush if pythonbuf is destroyed (a problem revealed by the added tests). I dropped a bit of unneeded verbosity, as well.

henryiii · 2017-08-23T03:43:34Z

I think this is ready, finished polishing it off.

Primarily due to the usefulness of call_guard, a scoped_estream_redirect has been added with cerr defaults. This allows both cout and cerr to be redirected easily:

m.def("noisy_function", &noisy_function,
      py::call_guard<py::scoped_ostream_redirect,
                     py::scoped_estream_redirect>());

This also simplified some other code without losing generality. Other changes are mostly cosmetic; Docs have been nicely formatted and expanded a bit, etc.

jagerman · 2017-08-23T13:23:08Z

This branch cannot be rebased due to conflicts
Rebasing the commits of this branch on top of the base branch cannot be performed automatically due to conflicts encountered while reapplying the individual commits from the head branch.

Can you squash the commits down to one or two commits with appropriate merged/updated commit messages, and rebase against current master (rather than merging the current master branch)? (That will keep the master commit history much cleaner when this gets merged).

henryiii · 2017-08-23T14:15:23Z

Rebased, squashed down to 5 commits that each had a valid, working version. (I can merge to fewer if needed, or squash-and-merge should work now).

henryiii · 2017-08-23T22:39:47Z

Finally the Mac build finished.

henryiii · 2017-08-24T06:20:54Z

Missed an inline (the usage of py::class_ is in a function, not defining a class), re-pushed.

dean0x7d · 2017-08-24T13:15:36Z

@henryiii Our sphinx setup is a bit peculiar so I went over and fixed up some reference links and added a section/links to the API docs. Minor (whitespace) style changes in the header.

Only significant bits:

add_ostream_redirect now adds the class as module_local to avoid module conflicts.
Dropped scoped_estream_redirect constructors in favor of inherited ones.

Let me know if anything looks off here. If not, I think this PR can be merged.

Squashed commit of the following: commit 3f55f7c Author: Henry Fredrick Schreiner <[email protected]> Date: Fri Aug 18 09:35:47 2017 -0400 Fix short underline in docs commit bc8ac7d Author: Henry Fredrick Schreiner <[email protected]> Date: Fri Aug 18 08:53:15 2017 -0400 Typo fix in test commit e53f073 Author: Henry Fredrick Schreiner <[email protected]> Date: Fri Aug 18 08:43:12 2017 -0400 Linting fixes commit d8f47fb Author: Henry Fredrick Schreiner <[email protected]> Date: Fri Aug 18 00:13:43 2017 -0400 Linting fixes commit 84fe118 Author: Henry Fredrick Schreiner <[email protected]> Date: Fri Aug 18 00:11:25 2017 -0400 Docs typo fixed from lintig commit 3187674 Author: Henry Fredrick Schreiner <[email protected]> Date: Fri Aug 18 00:03:11 2017 -0400 Fixing Pypy tests commit 6a97ecc Author: Henry Fredrick Schreiner <[email protected]> Date: Thu Aug 17 23:26:16 2017 -0400 Minor cleanup from style lint commit c1c14c9 Author: Henry Fredrick Schreiner <[email protected]> Date: Thu Aug 17 22:56:35 2017 -0400 Changelog update commit 57d1d88 Author: Henry Fredrick Schreiner <[email protected]> Date: Thu Aug 17 22:53:37 2017 -0400 Adding tests for psudo-global iostream wrapping commit bcb6cc8 Author: Henry Fredrick Schreiner <[email protected]> Date: Thu Aug 17 22:34:08 2017 -0400 Adding docs and triple-test commit 54993c9 Author: Henry Fredrick Schreiner <[email protected]> Date: Thu Aug 17 22:21:08 2017 -0400 Adding method suggested in pybind#1005

Squashed commit of the following: commit 94d65fb040bf10eea9cda3093e73d87ec2c503e9 Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 23:52:38 2017 -0700 Fixing refer error commit 3ba58dc110d3f313e9176f5f17830e903f8f41c8 Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 23:15:51 2017 -0700 Fix typo in windows fix commit 0100145a240187f8d97b348daada162f1808de5f Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 23:06:37 2017 -0700 Fix header not linking in docs commit 2a8273df216143fbb42e104b46d129b00146cda2 Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 23:04:10 2017 -0700 Fixing usage of reserved word on windows commit caefb6c8d8bf3f362dd871b6e94343a226f2585a Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 22:37:34 2017 -0700 Removing output in logs, better testing commit 315645e3883b8ee801af9ccedf6d6cdf0f092a60 Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 21:43:05 2017 -0700 Renames and cleanup commit 3841c79a3914ffb32bf0b6192c9d06acbe2e8d57 Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 14:43:52 2017 -0700 Typo fix commit 861478aae97ee09bfd9675745ce736a5c4f9ffac Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 12:49:57 2017 -0400 Using setup function commit 8c718359c780c248c697b9354cda83b691203bb7 Author: Henry Fredrick Schreiner <[email protected]> Date: Sun Aug 20 10:03:54 2017 -0400 Removing globaliostream, combined from others commit 55e372f74ba73fce2c763c112d70036a8ea3ec4b Author: Henry Fredrick Schreiner <[email protected]> Date: Sat Aug 19 20:32:17 2017 -0400 Move to context manager method commit 7738d2fc7c47b1bd652f03a3c3246672a049934b Author: Henry Fredrick Schreiner <[email protected]> Date: Sat Aug 19 17:15:26 2017 -0400 Addressing first four comments

@dean0x7d

Squashed commit of the following: commit a310fa30069de7de0b92019e7736939d52fb725e Author: Henry Fredrick Schreiner <[email protected]> Date: Mon Aug 21 14:50:49 2017 -0700 Dropping a class and simpler code, from @dean0x7d commit 2af3855adb781716ed22db1aa07e09226aff3d1f Author: Henry Fredrick Schreiner <[email protected]> Date: Mon Aug 21 14:47:26 2017 -0700 Add license to header

Squashed commit of the following: commit 4cdbdd3809cae488107c33838eb4dd2346de73f8 Author: Henry Fredrick Schreiner <[email protected]> Date: Tue Aug 22 10:12:23 2017 -0700 Linting fix commit 4ff64ce2ebf9b26c60575f8edb93b488f4677431 Author: Henry Fredrick Schreiner <[email protected]> Date: Tue Aug 22 08:34:38 2017 -0700 Docs cleanup, changelog update commit 00cc177c2e1148b71000e9564f8fcf4947f0b8f5 Author: Henry Fredrick Schreiner <[email protected]> Date: Tue Aug 22 08:21:35 2017 -0700 Adding scoped_estream_redirect, for call_guard and simplification commit 30899eaeba30ff453338040c93a221879351efb7 Author: Henry Fredrick Schreiner <[email protected]> Date: Tue Aug 22 07:33:47 2017 -0700 Docs cleanup and update commit fe1a63a125330a01684ed8a26d77523195c90735 Author: Henry Fredrick Schreiner <[email protected]> Date: Mon Aug 21 17:36:17 2017 -0700 Fix linter issues commit e2b8f8ad6abcac4864f78c93f8b89e85215aa0a0 Author: Henry Fredrick Schreiner <[email protected]> Date: Mon Aug 21 16:47:44 2017 -0700 Dropping some extra unneeded thises and unused default commit 2e8bec13e94dcba71693788200e4ed7c40e2ac60 Author: Henry Fredrick Schreiner <[email protected]> Date: Mon Aug 21 16:43:41 2017 -0700 Sync on destuction, and add tests to check flush behavior commit 314d4514685a3f17a13da5864c1f055d66688458 Author: Henry Fredrick Schreiner <[email protected]> Date: Mon Aug 21 16:11:52 2017 -0700 Testing and documenting simple call with call_guard

Also added `module_local` to `add_ostream_redirect`.

dean0x7d · 2017-08-24T14:43:48Z

There was also a missing header in setup.py. I rebased the PR to have the new installation tests cover it. The commit order is fine offline, but for some reason, GitHub decided to randomize them...

henryiii · 2017-08-24T18:16:35Z

The module_local was new in 2.2, so I didn't know about it. I checked the scoped_estream_redirect constructors, the new version seems to work correctly. (I was overly paranoid, but deleted constructors and assignments stay deleted).

Looks good, thanks!

dean0x7d · 2017-08-25T00:21:26Z

Merged! Thanks, @henryiii!

The module_local was new in 2.2, so I didn't know about it.

It was added very recently, so I also didn't remember to add it initially, but it looks like a good fit for add_ostream_redirect.

henryiii · 2017-08-25T01:02:50Z

Great, thanks for the help polishing! Looking forward quite a few of the features in 2.2!

dean0x7d requested changes Aug 19, 2017

View reviewed changes

dean0x7d approved these changes Aug 21, 2017

View reviewed changes

dean0x7d modified the milestone: v2.2 Aug 23, 2017

henryiii force-pushed the stdout branch from 48934e4 to 5d44c1a Compare August 23, 2017 14:12

dean0x7d mentioned this pull request Aug 23, 2017

Make a 2.2 release #953

Closed

6 tasks

henryiii force-pushed the stdout branch from 5d44c1a to f748ed7 Compare August 24, 2017 06:18

dean0x7d force-pushed the stdout branch from 1d810f9 to 886751d Compare August 24, 2017 13:19

henryiii added 3 commits August 24, 2017 16:30

Adding much simpler version of opythonstream from @jagerman

0c34c94

henryiii and others added 4 commits August 24, 2017 16:30

Add section to reference docs and minor style changes

d81cd80

Also added `module_local` to `add_ostream_redirect`.

Fix missing header in setup.py

a45d25e

dean0x7d force-pushed the stdout branch from 886751d to a45d25e Compare August 24, 2017 14:31

dean0x7d merged commit 8b40505 into pybind:master Aug 25, 2017

henryiii deleted the stdout branch August 25, 2017 00:57

henryiii mentioned this pull request Aug 29, 2017

WIP: Changes preparing for Lineshapes GooFit/GooFit#114

Merged

rwgk mentioned this pull request Feb 9, 2023

FWD pybind11 google/pybind11clif#1009

Closed

fangq mentioned this pull request Aug 26, 2023

[BUG]: Unable to show stdout output of a binary module inside Jupyter notebook #4818

Open

3 tasks

Stdout redirection #1009

Stdout redirection #1009

Conversation

henryiii commented Aug 18, 2017 • edited Loading

Capturing standard output from ostream

henryiii commented Aug 18, 2017

aldanor commented Aug 18, 2017

henryiii commented Aug 18, 2017

henryiii commented Aug 18, 2017 • edited Loading

dean0x7d left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henryiii commented Aug 20, 2017 • edited Loading

henryiii commented Aug 20, 2017 • edited Loading

jagerman commented Aug 20, 2017

dean0x7d commented Aug 20, 2017 • edited Loading

henryiii commented Aug 21, 2017

jagerman commented Aug 21, 2017

dean0x7d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dean0x7d Aug 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dean0x7d commented Aug 21, 2017

henryiii commented Aug 21, 2017

henryiii commented Aug 21, 2017

henryiii commented Aug 23, 2017

jagerman commented Aug 23, 2017

henryiii commented Aug 23, 2017 • edited Loading

henryiii commented Aug 23, 2017

henryiii commented Aug 24, 2017 • edited Loading

dean0x7d commented Aug 24, 2017

dean0x7d commented Aug 24, 2017

henryiii commented Aug 24, 2017

dean0x7d commented Aug 25, 2017

henryiii commented Aug 25, 2017

henryiii commented Aug 18, 2017 •

edited

Loading

henryiii commented Aug 18, 2017 •

edited

Loading

dean0x7d left a comment •

edited

Loading

henryiii commented Aug 20, 2017 •

edited

Loading

henryiii commented Aug 20, 2017 •

edited

Loading

dean0x7d commented Aug 20, 2017 •

edited

Loading

dean0x7d Aug 21, 2017 •

edited

Loading

henryiii commented Aug 23, 2017 •

edited

Loading

henryiii commented Aug 24, 2017 •

edited

Loading