code cleanup#790
Conversation
ensures that operator<< writes to the correct ostream fixes python bindings removes comments of the form // cout << ... fixes and orders includes
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cba2cb15cb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #790 +/- ##
==========================================
- Coverage 29.22% 29.21% -0.01%
==========================================
Files 241 242 +1
Lines 35561 35566 +5
Branches 14824 14829 +5
==========================================
- Hits 10394 10392 -2
- Misses 17941 17947 +6
- Partials 7226 7227 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| */ | ||
| void print() const { | ||
| this->_impl->print(std::cout); | ||
| std::cout << std::string(" Scalar dtype: [") << Type.getname(this->_impl->_dtype) | ||
| << std::string("]") << std::endl; | ||
| void print() const { this->print(std::cout); } | ||
| /// @cond | ||
| void print(std::ostream &os) const { | ||
| this->_impl->print(os); | ||
| os << std::string(" Scalar dtype: [") << Type.getname(this->_impl->_dtype) << std::string("]") | ||
| << std::endl; | ||
| } | ||
| /// @endcond |
There was a problem hiding this comment.
What is the reason to add new methods which consume ostream?
There was a problem hiding this comment.
std::ostream &operator<<(std::ostream &os ...) functions call the print functions. Previously, those wrote to std::cout, no matter what the ostream in operator<< was. I fixed this. To make all IO options uniform (print(), print_...() etc), I added a version that accepts an ostream always.
There was a problem hiding this comment.
I feel a better pattern is creating a new method that compose the output string, and let Scaler::std::ostream &operator<<(std::ostream &os ...) call that method if std::ostream &operator<<(std::ostream &os ...) exist. With the pattern, print() is useless and can be removed.
There was a problem hiding this comment.
We could directly implement it in operator<< (and print() in impl). I outlined it in #868.
|
You did at least four things in one commit.
To make a PR review by reviewers easier and more quickly, developers are asked to split their great works into multiple commit by logical feature. You can find many sources talking about this principle and try to review PRs. 5.2 Distributed Git - Contributing to a Project
Linux kernel submitting guidelines
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4d2e135bb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08d7678a1a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c7407b253
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
3 participants
ensures that
operator<<writes to the correctostreamfixes python bindings
removes comments of the form
// cout << ...fixes and orders includes