Skip to content

Feature/np cumsum#307

Merged
Nucs merged 3 commits intoSciSharp:masterfrom
Plankton555:feature/np_cumsum
Jul 3, 2019
Merged

Feature/np cumsum#307
Nucs merged 3 commits intoSciSharp:masterfrom
Plankton555:feature/np_cumsum

Conversation

@Plankton555
Copy link
Copy Markdown
Contributor

Simple implementation of cumulative sum. Either call as a member function from an NDArray or as a static function via np.cumsum().

@Plankton555 Plankton555 mentioned this pull request Jul 2, 2019
@Nucs Nucs self-requested a review July 2, 2019 13:48
Copy link
Copy Markdown
Member

@Nucs Nucs left a comment

Choose a reason for hiding this comment

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

Looking good, just missing some documentations.


public static partial class np
{
public static NDArray cumsum(NDArray a)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add documentation similar to this

Copy link
Copy Markdown
Contributor

@henon henon left a comment

Choose a reason for hiding this comment

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

Please always add the tests from the numpy docs examples too:

https://docs.scipy.org/doc/numpy-1.15.1/reference/generated/numpy.cumsum.html

Copy link
Copy Markdown
Member

@Oceania2018 Oceania2018 left a comment

Choose a reason for hiding this comment

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

@Plankton555 Glad you get started in NumSharp jounery. Just a little suggestion about this PR.
Try this Assert.IsTrue(Enumerable.SequenceEqual(arr, expected)); instead of for loop.

@Plankton555
Copy link
Copy Markdown
Contributor Author

@Oceania2018 I tried using Linq instead but didn't get it to work. Any advice are welcome

Assert.IsTrue(Enumerable.SequenceEqual(arr, expected));
resulted in
The type arguments for method 'Enumerable.SequenceEqual<TSource>(IEnumerable<TSource>, IEnumerable<TSource>)' cannot be inferred from the usage. Try specifying the type arguments explicitly

Assert.IsTrue(Enumerable.SequenceEqual<double>(arr, expected));
resulted in
cannot convert from 'NumSharp.NDArray' to 'System.Collections.Generic.IEnumerable<double>

@Oceania2018
Copy link
Copy Markdown
Member

Assert.IsTrue(Enumerable.SequenceEqual(arr.GetData<double>(), expected.GetData<double>()))

@Plankton555
Copy link
Copy Markdown
Contributor Author

  • Tests from the examples in the numpy documentation have been added.
  • Added documentation to functions.

4/6 tests are failing currently. The failing tests are related to summing over specific axes, as well as specifying data types.

@Nucs Nucs merged commit ab2ac4b into SciSharp:master Jul 3, 2019
@Nucs
Copy link
Copy Markdown
Member

Nucs commented Jul 3, 2019

Thank you for the PR!

@Plankton555 Plankton555 deleted the feature/np_cumsum branch July 3, 2019 08:40
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.

4 participants