Quansight-Labs / numpy.net

A port of NumPy to .Net
BSD 3-Clause "New" or "Revised" License
128 stars 14 forks source link

Fixed #19 #30

Closed Happypig375 closed 2 years ago

KevinBaselinesw commented 2 years ago

I can't accept these changes because this unit test fails. If you can fix that problem, I will consider accepting the change. Thank you.

    [TestMethod]
    public void test_ndarray_delete1()
    {
        var x = np.arange(0, 32, dtype: np.Int16).reshape(new shape(8, 4));
        print("X");
        print(x);
        print(x.shape);

        var ExpectedDataX = new Int16[8, 4]
        {
                { 0, 1, 2, 3},
                { 4, 5, 6, 7},
                { 8, 9, 10, 11 },
                { 12, 13, 14, 15},
                { 16, 17, 18, 19},
                { 20, 21, 22, 23},
                { 24, 25, 26, 27},
                { 28, 29, 30, 31},
        };

        AssertArray(x, ExpectedDataX);
        AssertShape(x, 8, 4);

        var y = np.delete(x, new Slice(null), 0).reshape(new shape(8, 3));
        y[1] = 99;
        print("Y");
        print(y);
        print(y.shape);

        var ExpectedDataY = new Int16[8, 3]
        {
                { 1, 2, 3},
                { 99, 99, 99},
                { 9, 10, 11 },
                { 13, 14, 15},
                { 17, 18, 19},
                { 21, 22, 23},
                { 25, 26, 27},
                { 29, 30, 31},
        };

        AssertArray(y, ExpectedDataY);
        AssertShape(y, 8, 3);

        print("X");
        print(x);

        AssertArray(x, ExpectedDataX);
        AssertShape(x, 8, 4);
    }
Happypig375 commented 2 years ago

Is it a correct test in the first place? The corresponding python code in file ArrayCreationTests.py is

import numpy as np
x = np.arange(0,32, dtype= np.int16).reshape(8,4)
print("X")
print(x)
print(x.shape)

y = np.delete(x, 0, axis=1)
y[1] = 99
print("Y")
print(y)
print(y.shape)

print("X")
print(x)

and by logic it does not make sense how deleting along axis 0 (rows) will delete a column. Perhaps it's the test that should be fixed?

Happypig375 commented 2 years ago

Test updated

KevinBaselinesw commented 2 years ago

I have accepted your changes and merged them into master. Thank you again.

Happypig375 commented 2 years ago

You're welcome!