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.

Sign up for GitHub

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

Jump to bottom

add ndslice.slice.as #4825

Merged
merged 1 commit into from Oct 1, 2016
Merged

add ndslice.slice.as #4825

merged 1 commit into from Oct 1, 2016

Conversation

9il
Copy link
Member

@9il 9il commented Sep 30, 2016 •
edited

Motivation:

  1. most ndslice users would be scientists without engineering experience. ndslice should provide a set of simple concepts, that will help scientists to start use it. Otherwise we will lose our potential users.
  2. It is optimised to reduce template bloat in case of the identical type

@9il 9il added the ndslice label Sep 30, 2016
@9il 9il mentioned this pull request Sep 30, 2016
Closed
wilzbach
wilzbach reviewed Sep 30, 2016
View reviewed changes
Copy link
Member

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

This looks nice :)
Added tiny nits.

As this is a API name addition @andralex should get consulted too.
For my part the name "as" is very common, e. g. NumPy uses it too.

std/experimental/ndslice/slice.d
Params:
slice = a slice to create a view on.
Returns:
a lazy slice with elements converted to type `T`.
Copy link
Member

Choose a reason for hiding this comment

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

nit: start paragraph with upper case. a -> A

std/experimental/ndslice/slice.d
@@ -919,6 +920,56 @@ unittest
assert(shape[1] == 0);
}


Copy link
Member

Choose a reason for hiding this comment

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

Nit: two empty lines

@wilzbach wilzbach added the @andralex Approval from Andrei is required label Sep 30, 2016
@9il
Copy link
Member Author

9il commented Sep 30, 2016

@wilzbach Thanks, fixed

@andralex
Copy link
Member

andralex commented Oct 1, 2016

cool as frack

@andralex andralex merged commit d387277 into dlang:master Oct 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wilzbach wilzbach wilzbach left review comments

Assignees
No one assigned
Labels
@andralex Approval from Andrei is required
Projects
None yet
Milestone
No milestone
3 participants
@9il @andralex @wilzbach