removed force/check option for put_block#702
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #702 +/- ##
==========================================
+ Coverage 32.35% 35.33% +2.98%
==========================================
Files 215 215
Lines 36363 33020 -3343
Branches 14597 13132 -1465
==========================================
- Hits 11764 11669 -95
+ Misses 22659 19437 -3222
+ Partials 1940 1914 -26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This breaks Python API. I suggest deprecate the API first if we are not planning to release version 2. |
Since there is no use case for this (see #501), I do not think than this option was really used. If we still want to make the API deprecated first, how would this work? Do we have a standard procedure that warns users when they use a feature that is deprecated? |
The function signature is changed. If someone use it, their code will be broken.
You can refer how popular packages did or #672. |
done |
pybind/unitensor_py.cpp
Outdated
| .def( | ||
| "put_block_", | ||
| const bool &force) { | ||
| py::warnings::warn("argument 'force' is deprecated and will be removed; use put_block((UniTensor &self, cytnx::Tensor &in, const std::vector<std::string> &lbls, const std::vector<cytnx_int64> &qnum) instead", |
There was a problem hiding this comment.
This will be warned in Python interface where the users don't know C++ APIs.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38bbd295e5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| put_block(const Tensor &in_tens, const std::vector<cytnx_int64> &qidx, const bool &force) { | ||
| this->_impl->put_block(in_tens, qidx); |
There was a problem hiding this comment.
Honor deprecated force argument in put_block overload
This deprecated overload still exposes force, but it now unconditionally calls the strict two-argument path and ignores the flag. In previous behavior, force=false allowed callers to skip missing blocks without throwing; after this change, the same call path will throw, which is a breaking behavioral change hidden behind a deprecation shim. If the overload is kept for compatibility, it should preserve force semantics (or explicitly reject force=false instead of silently changing behavior).
Useful? React with 👍 / 👎.
| put_block_(Tensor &in, const std::vector<cytnx_int64> &qidx, const bool &force) { | ||
| this->_impl->put_block_(in, qidx); |
There was a problem hiding this comment.
Honor deprecated force argument in put_block_ overload
The deprecated in-place overload also ignores its force argument by directly forwarding to the strict two-argument implementation. Existing code that used force=false as a non-throwing update path will now raise exceptions after this commit, despite still calling a valid deprecated API. The compatibility overload should either preserve the old force behavior or fail fast with a clear error when force=false is supplied.
Useful? React with 👍 / 👎.
Fixes #501