Added remaining Numpy NDArray single function expressions #183
Conversation
…s: numpy.linalg.{matrix_power, qr, svd, det, matrix_rank, inv, pinv}.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Hey @aritrakar, thanks for proposing the change! Please sign the CLA to proceed the PR. |
Hey @odashi , nice to hear from you. I'd signed the CLA after I saw the error. Do I need to sign a new one? |
|
@aritrakar Ah I see, maybe I'd overlooked something. |
|
@aritrakar It looks some CIs are still failing. You can check them locally by running |
|
Hey @odashi , sorry about that. I fixed the issues related to line lengths. I checked the CI tests locally this time and there seem to be some issues in files that I haven't touched. Do you know what's going on? For reference, this is what I see when I run
|
|
@aritrakar It looks you are using Python older than 3.10. the Match syntax was introduced by that version and any versions older than 3.10 can't process the syntax. |
|
Right. I've upgraded to use Python 3.11.6 now and the errors related to Match have disappeared. However, the errors related to flake8 and mypy seem to persist and looks like they're common across PRs. For reference:
|
|
@aritrakar Yeah They are common as I mentioned in another PR: I will handle this error on this PR as well. |
|
|
||
| func_arg = node.args[0] | ||
| if isinstance(func_arg, ast.Name): | ||
| return rf"\det \left( \mathbf{{{func_arg.id}}} \right)" |
There was a problem hiding this comment.
It looks some generation rules don't follow the convention of the library: \mathopen{}\left* and \mathclose{}\right*
| def _generate_qr_and_svd(self, node: ast.Call) -> str | None: | ||
| """Generates LaTeX for numpy.linalg.qr and numpy.linalg.svd. | ||
| Args: | ||
| node: ast.Call node containing the appropriate method invocation. | ||
| Returns: | ||
| Generated LaTeX, or None if the node has unsupported syntax. | ||
| Raises: | ||
| LatexifyError: Unsupported argument type given. | ||
| """ | ||
| name = ast_utils.extract_function_name_or_none(node) | ||
| assert name == "QR" or name == "SVD" | ||
|
|
||
| if len(node.args) != 1: | ||
| return None | ||
|
|
||
| func_arg = node.args[0] | ||
| if isinstance(func_arg, ast.Name): | ||
| func_arg_str = rf"\mathbf{{{func_arg.id}}}" | ||
| return rf"\mathrm{{{name.upper()}}} \left( {func_arg_str} \right)" | ||
|
|
||
| elif isinstance(func_arg, ast.List): | ||
| matrix_str = self._generate_matrix(node) | ||
| return rf"\mathrm{{{name.upper()}}} \left( {matrix_str} \right)" | ||
| return None |
There was a problem hiding this comment.
This function can be removed. The current implementation converts QR(A) to SVD(A) to
| return rf"\mathrm{{{name.upper()}}} \left( {matrix_str} \right)" | ||
| return None | ||
|
|
||
| def _generate_inverses(self, node: ast.Call) -> str | None: |
There was a problem hiding this comment.
It looks there is no reason to combine inv and pinv into the same function.
There was a problem hiding this comment.
To clarify, would you rather I separate them into two different functions?
There was a problem hiding this comment.
Yeah that would be reasonable I think.
|
@aritrakar Merging |
|
@aritrakar Thanks for a bunch of effort! |
Overview
This pull request expands support for several NumPy linear algebra functions involving a single operand (as mentioned in #149), including
numpy.linalg.{matrix_power, qr, svd, det, matrix_rank, inv, pinv}. It introduces support for applying these functions to both variables and NDArrays, improving the versatility of the codebase.Details
Key changes in this PR:
transposeimplementation.References
#149
Blocked by
N/A