Qobj: Deprecate Qobj @ numpy.array#2829
Conversation
There was a problem hiding this comment.
Thank you for the contribution. Other than the comment below, this seems like a good addition. (@Ericgig is there any reason not to add this / why it hasn't been added yet?)
Note also, the changelog file should not have the ".rst" ending; that's why the documentation build is failing.
| ) | ||
|
|
||
| def __rmatmul__(self, other) -> Qobj: | ||
| if not isinstance(other, Qobj): |
There was a problem hiding this comment.
This condition should always be true: if other were a Qobj, then it would have tried other.__matmul__(self) first. I would suggest to just remove the condition.
|
Do we want to encourage / officially support operations between If we do, what about If not, should we remove it from |
It's an a bit difficult question. In principle, I would be for removing it altogether, but I don't know if that could be a breaking change for users. However, having it only for the one order of operands is definitely weird. @Ericgig what do you think is best? (And sorry @mudit06mah that #2547 was maybe somewhat misleading for an open issue.) |
No worries, Since there is still need for changes (whether to add support or remove it completely) so I don't think it was invalid to keep it open. |
|
Personally I am for not supporting any operations with numpy array with the present Qobj. But for this kind of design decision, it will be better to discuss with the other qutip developers, we will meet in a few weeks. |
| @@ -0,0 +1 @@ | |||
| Added a __rmatmul__() function to Qobj class to successfully handle numpy.ndarray @ Qobj operations | |||
There was a problem hiding this comment.
Would it be considered a bug fix or a feature, though?
I added a new label ( |
|
@mudit06mah: Thank you for raising this issue. As mentioned in #2547 we decided to deprecate support for numpy arrays in You're welcome to make a PR for the deprecation if you like, otherwise one of the admin team will get to it. |
Sure, I'd change this PR only to deprecate it instead :) |
5f7bd08 to
4eda31b
Compare
|
@hodgestar @Ericgig @pmenczel @veronikakurth I have changed this PR to deprecate the operation instead :D PTAL |
| if isinstance(other, np.ndarray): | ||
| warnings.warn( | ||
| "Support for Qobj @ numpy.array has been deprecated " | ||
| "and will be removed in qutip 5.4. " |
There was a problem hiding this comment.
It might be a bit of a short timeline for deprecation; @Ericgig would you say it would be better to communicate that the support will be removed in "qutip 5.5 or later"?
There was a problem hiding this comment.
I have changed the warning message, Could you please review it again?
| "Support for Qobj @ numpy.array has been deprecated " | ||
| "and will be removed in qutip 5.5 or later. " | ||
| "Please use Qobj(A) @ B instead.", | ||
| DeprecationWarning, |
There was a problem hiding this comment.
It's better to use FutureWarning, DeprecationWarning are hidden per default so the average user will miss them.
There was a problem hiding this comment.
I see, I changed the stack level to 2 to make the DeprecationWarning visible earlier. I have changed it to FutureWarning now.
Description
This PR deprecates
Qobj @ numpy.arrayoperation.Related issues or PRs
fixes #2547
Steps to test
Code:
Output: