DOC: Update numpy4matlab #17159
DOC: Update numpy4matlab #17159
Conversation
There was a problem hiding this comment.
Thanks for that @cooperrc ! I left a number of comments, some of them very minor, some only suggestions. One think I'd like to point out is that there are several pieces of Python code in that document non-PEP8 compliant, mainly around slicing and indexing operations.
There are also some places where MATLAB has a registered symbol and other places where it doesn't. Might be worthwile mentioning in the beginning but dropping it for the rest of the text? I'm not sure how that can be done correctly.
I also liked your extra document about creating arrays, and I think it's definitely worth adding. There are also some details to be fixed there (like PEP8 spacing and some wording).
Thanks again for doing this, I know it's a large document with many details but I think this is a very important document for those looking to make the switch.
@melissawm Thanks for pointing this out. Google style guide says
MathWorks says
and
|
If you intend it as part of this PR, please go ahead and include it. |
One option we can consider is splitting this document into two: one more of a "cheatsheet", mostly containing the table currently in this document, and another with more details and examples of how to do things properly in NumPy. This can be considered if the document get too large or if we feel there may be two separate audiences (I suspect that might be the case). But this can be a next step after this PR is merged. |
I like this idea, especially if cells in the former link to the latter. Although a single document would work well too now that we have nice sidebar navigation, as long as the table comes first. |
I wasn't sure if it directly closes the issue of updating the current document. It would certainly help a Matlab user, but the original issue was just to update the original document. This would add new material. What do you think? |
Thanks for reviewing and the careful edits. It is very much appreciated!
@bjnath found the correct way. Thanks!
I have missed a lot of PEP8 formatting. Thanks for catching those.
You're very welcome, I agree. This is a document I have referenced many times converting between NumPy and MATLAB. I'm happy to give back. |
@melissawm and @eric-wieser I like this idea too. It may even be worthwhile to consider a short tutorial on converting MATLAB to NumPy. We could pick an example that highlights some of the strengths in using NumPy over MATLAB. One thing that comes to mind is multidimensional arrays. In NumPy, you can nest lists and build N-D arrays (great for continuum mechanics and other tensor-based models), but in Matlab you have to recursively redefine the array to get more dimensions e.g. A=[1,2;3,4]
A(:,:,2)=[5,6;7,8]Vs NumPy A=np.array([[[1,2],[3,4]],
[[5,6],[7,8]]]) |
|
@cooperrc It might be worth making a commit now that incorporates @melissawm 's changes so we have a clean base for further review while we think about 2 vs 1. |
update ref for see note HELP L104 Co-authored-by: Melissa Weber Mendonça <melissawm@gmail.com>
update range reference Co-authored-by: Melissa Weber Mendonça <melissawm@gmail.com>
update Python boolean ref Co-authored-by: Melissa Weber Mendonça <melissawm@gmail.com>
give link for boolean in the short-circuit logical AND Co-authored-by: Melissa Weber Mendonça <melissawm@gmail.com>
|
@melissawm I run into a
I'm comparing functions, |
|
@cooperrc, are you referring to |
Yes, sorry about that: |
update `source` -> `np.source` L108 Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
update link for boolean Co-authored-by: Melissa Weber Mendonça <melissawm@gmail.com>
update links for `help` and `np.source` L583 Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
update references to boolean values instead of types Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
add link to cholesky Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
add missin `np.` to L360 Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Ben Nathanson <github@bigriver.xyz>
Co-authored-by: Ben Nathanson <github@bigriver.xyz>
incorporate @bjnath edits, nice suggestions Co-authored-by: Ben Nathanson <github@bigriver.xyz> Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
|
@eric-wieser, @bjnath, and @melissawm I think we hit all of the major changes that needed to go into this update. I'm really happy with this revision, thanks so much for your help! Can we table any further comments/suggestions and merge the PR? If there are other issues, I can open another PR to continue our discussion (specifically our thoughts on some the finer points on array dimensions in NumPy vs MATLAB). |
revert reference for MathWorks to original -> "The MathWorks, Inc." as seen on website Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
|
Thanks @cooperrc and all the reviewers. |
This PR closes #17094 that requests an update to the "NumPy for MATLAB users" restructured text document.
Comments were integrated from @melissawm, @bjnath, @rossbar, @8bitmp3, and @eric-wieser
Here is a brief list of changes:
arrayvsmatrixwas moved to the end of the document, since the general recommendation is to usearrayforandifstatements were added andrandomhas the updateddefault_rngrecommendation.