Creation of 1D cubic_curve fails with curve_factory

[chhinze]

Short description

While trying to create a cubic_curve with curve_factory, where the data is only of dimension 1, curve_factory fails systematically due to the later described problem.

How ro reproduce

# as new test case in file `splipy/curve_factory_test.py`
def test_cubicCurve1D(self):
        n_elems = 30
        t = np.linspace(0, 1, n_elems)
        # as column vector (reqired by splipy)
        x = (t**2).reshape(-1,1)

        # This one fails:
        CurveFactory.cubic_curve(x, CurveFactory.Boundary.FREE, t=t.tolist())

Problem origin:

Debugging the test case above leads to SplineObject.py:75, where the dimension is wrongly calculated as

self.dimension = self.controlpoints.shape[-1] - rational

This fails for 1D self.controlpoints, as shape[-1] returns the length of the vector instead of 1.

Idea for solution:

Since the shape of the controlpoints is determined in curve_factory.py:429 by cp = splinalg.spsolve(N,x) it seems to be enough to reshape the solution sp again by

# curve_factory.py:429-430
cp = splinalg.spsolve(N,x)
cp = cp.reshape(x.shape) # this is the new line

I was not sure, if this would break any other functionality, so it is proposed here, instead of making a pull request.

Versions:

• Splipy : 1.3.1 (commit cbb9db868b04cfd71e3d4b977dcc113180488f00)
• Numpy 1.16

[VikingScientist]

Thank you for finding this :smile:

Your proposed solution does indeed work. If you ever want to try and create a pull request on your own, then you install the test framework by

pip install pytest pytest-benchmark

and run all tests from the root directory by typing:

py.test splipy test_utils --benchmark-skip

If these all pass, then it is probably good for merging. As of numpy version 1.16, the tests fire of quite a few warnings (see #92), but hopefully we get this sorted out soon

I'll create a PR with your proposed solution, but if you like to add your name to the commit, then I'll wait a few days before merging (just recreate an identical PR from your user).

[VikingScientist]

Solution in PR #94

[chhinze]

Thank you for creating the pull request. For me it just matters, that the bug is fixed, not whose name is on it, so I am fine with you merging PR #94.

I ran the tests, but some of them failed (or warned at leat as you mentioned), so I was not sure, if my solution has any side effects and thought, that it would be better to just propose the solution. In the future, I will open proper PRs.

Thanks again for running such an awesome project!