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

Add quick start readme #785

Merged
merged 16 commits into from
Apr 21, 2020
Merged

Conversation

liufuyang
Copy link
Contributor

@liufuyang liufuyang commented Feb 22, 2020

As discussed here #778, I think this adding a README like this here can make beginners start with this lib much easier.

All the code in the doc is runnable in https://play.integer32.com/ playground :)

Fixes #778

@quietlychris
Copy link

I'm not a contributor to this crate, but having used ndarray in a limited capacity on some projects, everything in here looks good to me. I also found that it did a really good job at explaining some of the particular rough edges for beginners (like compilation errors due to type inference issues when calling Array::zeros()) that otherwise take some time to figure out.

@liufuyang
Copy link
Contributor Author

@quietlychris Thank you for the input, I am glad it can help. And this is just a starting point of a doc like this. I think if we have something like it then we can slowly make it even better, if see some frequent asked questions.

For now I feel a doc like this, could help attract new users, and combining other two docs it will probably be good enough for most cases when one wants to actually using this package :)

* `B @ &A` which consumes `B`, updates it with the result, and returns it
* `C @= &A` which performs an arithmetic operation in place

More info checkout https://docs.rs/ndarray/latest/ndarray/struct.ArrayBase.html#arithmetic-operations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
More info checkout https://docs.rs/ndarray/latest/ndarray/struct.ArrayBase.html#arithmetic-operations
For more info checkout https://docs.rs/ndarray/latest/ndarray/struct.ArrayBase.html#arithmetic-operations

```

### Deep Copy
As the usual way in Rust, a `clone()` call will
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be confusing: clone() will perform a deep copy of the array elements only if you are cloning an owned array (i.e. Array).
Cloning an ArrayView does not clone the underlying elements - you are just cloning the view reference (as it happens in Rust when cloning a & reference).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, I see. Let me add your explanation here then. Thank you very much :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd avoid the term "Deep Copy" altogether, since it's a bit confusing in Rust terms. Proposed heading is Copying Arrays

@LukeMathWalker
Copy link
Member

I left a bunch of style-related suggestions and a comment on wording, but overall it looks really good @liufuyang. Thanks for working on this!

It would be ideal to include a link to this quickstart guide in the README and in the landing page of the documentation, to make it discoverable.

@liufuyang
Copy link
Contributor Author

Thank you very much for the good input. I will see whether I can find time this weekend to address your inputs :)

@liufuyang
Copy link
Contributor Author

@LukeMathWalker Thank you for the corrections. I have updated as you suggested 👍

Co-Authored-By: Luca Palmieri <[email protected]>
@liufuyang
Copy link
Contributor Author

I added a few more lines showing usage of from_elem, as discussion mentioned here #778

Perhaps we can merge this soon? @jturner314 @LukeMathWalker :) ?

## Want to learn more?
Please checkout these docs for more information
* [`ArrayBase` doc page](https://docs.rs/ndarray/latest/ndarray/struct.ArrayBase.html)
* [`ndarray` for `numpy` user doc page](https://docs.rs/ndarray/0.13.0/ndarray/doc/ndarray_for_numpy_users/index.html)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both links can link to the latest version - use latest, * or similar for the version?

[2, 3]]
```

Noticing that `clone()` will perform a deep copy of the array elements only if you are cloning an owned array (i.e. `Array`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep copy is not strictly true. When using clone it's always the shallowest copy that is allowed by ownership rules. That means that an array of Rc<T> elements, will .clone() each element (i.e increase its refcount, not "deep copy")

Copy link
Member

@bluss bluss Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could explain that .clone() also copies the array's elements, it creates an independently owned array of the same type. Just another nice way the type system can tell you clearly what's happening.

As in Rust we have owner ship, so we cannot simply
update an element of an array while we have a
shared view of it. This will help us write more
robust code than numpy.
Copy link
Member

@bluss bluss Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we say just robust code, not talk down on numpy here - we don't have to make this comparison

@bluss
Copy link
Member

bluss commented Apr 13, 2020

Thanks for working on this

@liufuyang
Copy link
Contributor Author

@bluss Thank you for the input. I have updated the part you commented. Please take a look again? Thank you.

@bluss
Copy link
Member

bluss commented Apr 20, 2020

Great, we should merge this. It looks like you'd want to rebase/squash this before merge. Do you? Otherwise we can just merge it in

@bluss
Copy link
Member

bluss commented Apr 20, 2020

I have edited the PR description to put in the literal Fixes #778, so that the issue is closed when this is merged. link with keyword

@liufuyang
Copy link
Contributor Author

liufuyang commented Apr 21, 2020

@bluss Thank you. I think you can just use that Squash and merge button? :)
image

Something like this, then there is only a single commit for this in master branch

@bluss
Copy link
Member

bluss commented Apr 21, 2020

Ok, let's do that. I usually prefer to keep history if it's neat, but it should even keep all the authors. Let's go. Thanks again. (Otherwise a manual squash and cleanup is what I usually do.)

@bluss bluss merged commit 2113b7a into rust-ndarray:master Apr 21, 2020
@bluss
Copy link
Member

bluss commented Apr 21, 2020

I want to add that the preferred merge strategy of this repo is Merge commit, then squash. Not using rebase.

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

Successfully merging this pull request may close these issues.

Better doc for beginners?
4 participants