print functions can output to any stream#869
Conversation
operator<<(std::ostream file = std::cout) handles full output pybind uses print(obj, file) print_diagram, print_info etc have file as an optional argument, both on C++ and Python sides
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors printing and representation methods across various classes (such as Gncon, Network, Symmetry, UniTensor, Scalar, and Storage) to accept a customizable std::ostream destination, defaulting to std::cout. It also updates the corresponding Python bindings to support writing to file-like objects and improves repr implementations using std::ostringstream. The review feedback highlights critical compilation issues in the Python bindings, specifically the use of a non-existent <pybind11/warnings.h> header and an invalid py::warnings::warn API in pybind/scalar_py.cpp. Additionally, a definition mismatch was identified in src/BlockUniTensor.cpp, where beauty_print_block was incorrectly defined as a free function with a default argument instead of a const member function.
| #include <pybind11/functional.h> | ||
| #include <pybind11/warnings.h> |
There was a problem hiding this comment.
The header <pybind11/warnings.h> does not exist in standard pybind11. Including it will cause a compilation error. Please remove this include.
#include <pybind11/functional.h>| py::warnings::warn( | ||
| "Scalar.print() is deprecated; use `print(scalar)` (or `print(scalar, file=...)`).", | ||
| PyExc_FutureWarning, 2); |
There was a problem hiding this comment.
py::warnings::warn is not a valid pybind11 API. To raise a deprecation warning in Python from C++, use the standard Python C API function PyErr_WarnEx with PyExc_FutureWarning.
PyErr_WarnEx(
PyExc_FutureWarning,
"Scalar.print() is deprecated; use 'print(scalar)' (or 'print(scalar, file=...)').",
2);| void beauty_print_block(const cytnx_uint64 &Nin, const cytnx_uint64 &Nout, | ||
| const std::vector<cytnx_uint64> &qn_indices, | ||
| const std::vector<Bond> &bonds, const Tensor &block) { | ||
| const std::vector<Bond> &bonds, const Tensor &block, | ||
| std::ostream &file = std::cout) { |
There was a problem hiding this comment.
The function beauty_print_block is declared as a const member function of BlockUniTensor in include/UniTensor.hpp. However, here it is defined as a free function without the BlockUniTensor:: prefix and without the const qualifier. Additionally, default arguments should only be specified in the function declaration, not in the definition. Please define it as a member function and remove the default argument.
void BlockUniTensor::beauty_print_block(const cytnx_uint64 &Nin, const cytnx_uint64 &Nout,
const std::vector<cytnx_uint64> &qn_indices,
const std::vector<Bond> &bonds, const Tensor &block,
std::ostream &file) const {
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #869 +/- ##
==========================================
- Coverage 29.23% 29.19% -0.04%
==========================================
Files 241 241
Lines 35559 35607 +48
Branches 14822 14862 +40
==========================================
Hits 10394 10394
- Misses 17941 17978 +37
- Partials 7224 7235 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Code ReviewVerdict: the design is right and we'd like it in — but the diff itself is no longer rebasable; requesting a staged redo on current master. The design is good (and worth adopting as the repo pattern)
The practical problem: this May-31 diff patches code that no longer existsSince it was written, the 2026-07 refactor campaign rewrote most of its 29 files wholesale:
So the Suggested path
To be explicit so the effort isn't wasted: the direction is accepted — #868 is worth implementing, and this PR's API/pybind shape is the one we want. It's only the mechanical base that has moved out from under it. Posted by Claude Code on behalf of @pcchen |
2 participants
Separates the print part in #790 and implements #868.