On Wednesday, 16 December 2015 at 11:01:25 UTC, Robert burner
Schadek wrote:
Yes with many conditions:
* Documentation
** The documentation needs a complete rewrite. If I hadn't had
any prior knowledge, I would have needed to read the numpy
documentation to figure what this package does. That is not
acceptable. It is also not clear how the functionally in the
package is supposed to work together, and how it interacts with
the rest of phobos.
I will add description of what this module dose and how it works
under the hood. In the same time I expect few articles from
another engineers about ndslice like this
http://dlang.org/intro-to-datetime.html . It is much better to
have explanation from different engineers.
** Params, Returns ...
Agreed.
* Style
** the source code does not look like phobos
** s/assert (/assert(/g
** s/unittest {/unittest\n{/g
** unittest properties should be on the same line as the
unittest keyword
** spaces between operators
** dfmt and some manuel work is your friend
Agreed all except spaces between operators. I would like to do
not use them in someY examples exactly for readability reasons.
* Testing
** most tests only use itoa, what about arrays what about
arrays with user defined types. That's properly trivial but
should be tested.
Didn't agree. Corresponding features for arrays and
std.container.array are tested. 98% of ndslice do not do anything
with data. Its change strides and lengths _only_.
** interaction with rest of phobos. Can I call map on a Slice?
If I can, it should be tested so that it still works after the
next release.
OK, I will add test for `map`. For other Phobos Slice is just an
Random Access Range and it is already tested for iota, arrays,
std.container.array, dummyranges .
Please note this DMD bug (see reduced example by John):
https://issues.dlang.org/show_bug.cgi?id=15441
* Miscellaneous
** string mixins. I think some of the string mixins can be
removed for something more readable/debuggable
**
I have not found examples where string mixins can be removed.
Please refer to particular example. The code for `sliced` and
`assumeSorted` looks ugly. But I don't know how is can be done
another way. Remove this functionality is an option.
Thank you and regards,
Ilya
