Feature parity of DynArray and Array

[andrepd]

This patch adds multiple functions in DynArray that are present in Array but not in the former. As of this patch, the only functions which exist in Array but not in DynArray are

• Sorts and bsearch
• backwards and of_backwards

which I'm not comfortable handling (maybe someone else can help).

Also, filter was reimplemented in a more efficient way.

Finally, I added a function upd (short for "update") for the common idiom

a.(i) <- f a.(i)

which should be faster (no duplicate bounds checking) and more convenient; for example instead of

set a i (incr @@ get a i)

one can write

upd a i incr.

Let me know what you think.

[UnixJunkie]

Can you break this into smaller PRs?
For example if you introduce new functions, introduce them one by one.
That will be easier and faster to review and hence several different maintainers might help.

[UnixJunkie]

Also, I don't know this module so it doesn't help.
I wonder what are the use cases for this module which I never used.

[UnixJunkie]

Maybe @murmour could help review this.

[UnixJunkie]

I am too lazy to do a large chunk of code review now (pretty much all the time, to be honest).
If there is a good will around, please help.
I never use this module, so I am not very motivated.
On the other hand, feature parity sounds nice.

[fccm]

I only read the diff commit, didn't test it, but it look like a good proposal.

Only filter and partition need more review because it is new implementations, most other functions are rather simple functions just to replicate functions from the Array module.

Anyway a lot of tests are provided, for most functions.

[fccm]

There are calls to Pervasives.invalid_arg that should be replaced by calls to Stdlib.invalid_arg.

[fccm]

Should we add preprocessing elements for Pervasives / Stdlib for backward compatibility?

[UnixJunkie]

Thanks for the review.
I will merge it and do the minor fix you suggest.
I don't know what you mean for Pervasives/Stdlib.
Please wait after the merge and send a new PR if needed.

[UnixJunkie]

@fccm you could ask to be a member of the batteries team on github.
I don't have the rights to add someone though.
It gives you a badge on your github profile.

[UnixJunkie]

Idealy, an @SInCE tag must be added with NEXT_RELEASE for all the new functions in the interface.

[UnixJunkie]

The ChageLog should be updated, with andrep as author and fcc as reviewer.

[UnixJunkie]

@fccm thanks a lot for the review.
I did a cursory one, spotting a sum and fsum problem.

[fccm]

You don't need to replace invalid_arg by failwith.
Just by Stdlib.invalid_arg

https://gist.github.com/fccm/8b6e0f3fa856a350823fbacd334a7a77

[fccm]

New functions seem to be: singleton, first, left, right, head, tail, fill, split, combine, filteri, partition, for_all, exists, mem, memq, index_of, find, findi, index_of, modify, modifyi, fold_lefti, fold_righti, reduce, rev, rev_in_place, max, min, min_max, avg, favg, iter2, iter2i, for_all2, exists2, map2, map2i, cartesian_product, range, Exceptionless.find, Exceptionless.findi, unsafe_upd, upd

Updated functions: filter,

[UnixJunkie]

'Stdlib.' will not compile on older ocaml versions.

[UnixJunkie]

related to #984

[fccm]

Then Pervasives.invalid_arg.

[UnixJunkie]

fixed in master

[UnixJunkie]

a PR to update the interface file would be appreciated

[fccm]

This is something I can do.

[fccm]

I did a cursory one, spotting a sum and fsum problem.

I just quickly tested sum and fsum, I don't see any problem.

Which problem did you found?

[UnixJunkie]

the submitted version would have crashed on an empty data structure