New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented function returning Wilkinson Matrix #20922
Conversation
✅ Hi, I am the SymPy bot (v161). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like: This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.8. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
@sylee957, please review |
sympy/matrices/common.py
Outdated
@@ -1208,6 +1208,51 @@ def entry(i, j): | |||
return kls.zero | |||
return kls._new(size, size, entry) | |||
|
|||
@classmethod | |||
def Wilkinson(kls, n): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def Wilkinson(kls, n): | |
def wilkinson(kls, n): |
I'd try to use lowercase for method names
sympy/matrices/common.py
Outdated
wminus = diag([i for i in range(-n, n + 1)], unpack=True) + D + D.T | ||
wplus = abs(diag([i for i in range(-n, n + 1)], unpack=True)) + D + D.T | ||
|
||
return wminus, wplus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I'd noted in #20911, you should consider the class of the output to match the class of the method it's invoked from
So you should check the returns are in the following classes
MutableDenseMatrix.wilkinson -> MutableDenseMatrix
MutableSparseMatrix.wilkinson -> MutableSparseMatrix
ImmutableDenseMatrix.wilkinson -> ImmutableDenseMatrix
ImmutableSparseMatrix.wilkinson -> ImmutableSparseMatrix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fixed this in latest commit.
>>> from sympy import MutableSparseMatrix, ImmutableDenseMatrix, ImmutableSparseMatrix, MutableDenseMatrix
>>> w_m, w_p = MutableDenseMatrix.wilkinson(1)
>>> type(w_m)
<class 'sympy.matrices.dense.MutableDenseMatrix'>
>>> w_m, w_p = MutableSparseMatrix.wilkinson(1)
>>> type(w_m)
<class 'sympy.matrices.sparse.MutableSparseMatrix'>
>>> w_m, w_p = ImmutableDenseMatrix.wilkinson(1)
>>> type(w_m)
<class 'sympy.matrices.immutable.ImmutableDenseMatrix'>
>>> w_m, w_p = ImmutableSparseMatrix.wilkinson(1)
>>> type(w_m)
<class 'sympy.matrices.immutable.ImmutableSparseMatrix'>
Brief description of what is fixed or changed
Implemented function returning Wilkinson matrix
Release Notes