Save and Load objects#778
Conversation
…ethods: to_binary(std::ostream) and from_binary(std::istream) Clearer separation: Save and Load handle file IO, while to_binary and from_binary only write to ostream or istream implementation classes provide to_binary_dispatch(std::ostream) and from_binary_dispatch(std::istream)
…ectly because the char* is not terminated by \0. Potentially illegal memory access
manuschneider
force-pushed
the
save_load
branch
from
f1d349c to
f45c7c8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #778 +/- ##
==========================================
- Coverage 30.07% 30.06% -0.01%
==========================================
Files 238 238
Lines 35405 35510 +105
Branches 14729 14765 +36
==========================================
+ Hits 10648 10677 +29
- Misses 17505 17573 +68
- Partials 7252 7260 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![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: c00d2384da
ℹ️ 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".
|
The C++ coding style we use uses PascalCase for function names. We should be consistent with it for the new functions especially the functions in the changing files have already follow the coding style. |
I checked the Cytnx API. We use one-word names, both upper- and lowercase ( But I suggest deviating from Google's style guide here, because I could not find any instance where we currently use PascalCase for functions or methods, and we should not introduce yet another convention for new functions. Another reason to deviate from Google style guide is that we want to have a consistent API on Python and C++. In Python, snake_case is recommended for function names, and therefore I would also suggest this for our API (and because we already use it a lot). I renamed some functions in this branch because they started with an underscore (which should be avoided) and did not have very meaningful names (e.g., We should discuss this in #836 instead. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64b25e4bc0
ℹ️ 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: 19ac5b06f8
ℹ️ 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: 13ad2a01c2
ℹ️ 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: b9a36200cf
ℹ️ 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".
|
This can be checked and merged, HDF5 will be addressed in another PR (#783). |
|
@manuschneider I plan to deal with this PR after we relase v1.1.0, do you agree? |
|
I will take this PR with milestone v1.2.0. |
3 participants
addresses #157 and #660
The Pickle support can already be checked and merged at this stage, and HDF5 can be implemented subsequently (check #660 for this).