Added support for numpy NDarray method. Added corresponding tests #135
Conversation
|
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. |
examples/examples.ipynb
Outdated
| { | ||
| "data": { | ||
| "text/latex": [ | ||
| "$$ \\displaystyle \\mathrm{numpy}(a) = \\begin{bmatrix} {1} & {2} & {3} & {4} \\\\ {5} & {6} & {7} & {8} \\\\ {9} & {10} & {11} & {12} \\end{bmatrix} $$" |
There was a problem hiding this comment.
The function name "numpy" is weird as it may overrides the package name (in some cases). Something else (e.g., myarray) is preferred.
There was a problem hiding this comment.
@odashi I renamed function into myNdarray.
examples/examples.ipynb
Outdated
| } | ||
| ], | ||
| "metadata": { | ||
| "kernelspec": { |
There was a problem hiding this comment.
Remove unnecessary metadata.
There was a problem hiding this comment.
I removed kernelspec from metadata.
| @@ -3,6 +3,7 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| import math | |||
| import numpy as np | |||
There was a problem hiding this comment.
This import requires additional dev dependency in pyproject.toml.
There was a problem hiding this comment.
Hi Odashi, I added numpy version in pyproject.toml, let me know if the version is suitable for the project.
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| import ast | |||
| import textwrap | |||
|
|
|||
| import numpy as np | |||
There was a problem hiding this comment.
Unused.
| import numpy as np |
There was a problem hiding this comment.
Removed numpy import from function_codegen_test.py
Sounds good Odashi. I added tests in both codegen_test.py and regression_test.py. Let me know if they fit into the existing style of testing as this is my first time contributing to this project. |
|
@LakeBlair Now #143 (follow-up of #118) was merged, could you update this pull request to follow the recent changes? |
@odashi I took a look at #143 and it seems that it handled ndarray and have corresponding test cases. I am not sure what more to add on this PR? |
|
@LakeBlair Oh yeah, I checked this PR again, and I guessed this is no longer necessary. So it'll be better to be closed anyway. Sorry for bothoring, but thanks for your contribution! |
Overview
After some investigations into #118 , I optimized the code and added unit testing in function_codegen.py, added test in regression_test.py, and added an example in example.py
Details
Reference
#78
Blocked By