Skip to content
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

Added documentation and minor changes to matrices module #20718

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lastcodestanding
Copy link
Contributor

References to other Issues or PRs

Brief description of what is fixed or changed

Added documentation to eigen.py, dense.py, and matrices.py

Added shape check condition to _eval_matrix_mul_elementwise. Would earlier result in empty rows if argument shapes mismatched.

Other comments

Release Notes

  • matrices
    • Added documentation
    • Added shape check to _eval_matrix_mul_elementwise to give error when performing element-wise multiplication of matrices with different shapes.

@sympy-bot
Copy link

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:

  • matrices

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.
#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
Added documentation to eigen.py, dense.py, and matrices.py

Added shape check condition to `_eval_matrix_mul_elementwise`. Would earlier result in empty rows if argument shapes mismatched.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* matrices
  * Added documentation
  * Added shape check to `_eval_matrix_mul_elementwise` to give error when performing element-wise multiplication of matrices with different shapes.
<!-- END RELEASE NOTES -->

sympy/matrices/dense.py Outdated Show resolved Hide resolved
sympy/matrices/eigen.py Outdated Show resolved Hide resolved
sympy/matrices/eigen.py Outdated Show resolved Hide resolved
@lastcodestanding lastcodestanding force-pushed the matricesDoc branch 2 times, most recently from 2234bb7 to 3f67c9b Compare January 3, 2021 12:23
@lastcodestanding
Copy link
Contributor Author

Can someone please help me understand why these tests are failing? I just implemented the suggested changes in documentation. Even tried to redo it in a single commit, etc. but it is a persistent failure.

@oscarbenjamin
Copy link
Contributor

Somewhere in the middle of the output is the error message:

/home/runner/work/sympy/sympy/sympy/matrices/matrices.py:docstring of sympy.matrices.matrices.MatrixEigen.eigenvals:49: WARNING: Unexpected indentation.
/home/runner/work/sympy/sympy/sympy/matrices/matrices.py:docstring of sympy.matrices.matrices.MatrixEigen.eigenvals:50: WARNING: Block quote ends without a blank line; unexpected unindent.
/home/runner/work/sympy/sympy/sympy/matrices/matrices.py:docstring of sympy.matrices.matrices.MatrixEigen.eigenvals:51: WARNING: Bullet list ends without a blank line; unexpected unindent.

You can also try building the docs yourself:
https://docs.sympy.org/latest/documentation-style-guide.html

@lastcodestanding
Copy link
Contributor Author

Thank you very much. I managed to fix it.

@oscargus
Copy link
Contributor

It would be useful with some tests for the introduced error. Also, there are now some conflicts that needs to be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants