Repository: statsmodels/statsmodels
Pull request: BUG: Fixed BSplines to match existing docs
The docstring for `BSplines` appears to have been copied from that of  `UnivariateCubicCyclicSplines`, as it specified the `df` and `degree` parameters to be integers, but in fact required them to be lists (raising a `TypeError: 'int' object is not subscriptable` if either was an integer)
- Fixed docstring to specify `{list[int], int}` instead
- Implemented support for integers by converting `df` and `degree` to lists if they aren't already

- Added `test_bsplines` parameterised test to test the creation of BSplines instances and the transformation of their original data

- [ ] closes #6914
- [ ] closes #6916 for 1-D ndarrays
- [ ] tests added / passed. 
- [ ] code/documentation is well formatted.  
- [ ] properly formatted commit message. See 
      [NumPy's guide](https://docs.scipy.org/doc/numpy-1.15.1/dev/gitwash/development_workflow.html#writing-the-commit-message). 

<details>


**Notes**:

* It is essential that you add a test when making code changes. Tests are not 
  needed for doc changes.
* When adding a new function, test values should usually be verified in another package (e.g., R/SAS/Stata).
* When fixing a bug, you must add a test that would produce the bug in master and
  then show that it is fixed with the new code.
* New code additions must be well formatted. Changes should pass flake8. If on Linux or OSX, you can
  verify you changes are well formatted by running 
  ```
  git diff upstream/master -u -- "*.py" | flake8 --diff --isolated
  ```
  assuming `flake8` is installed. This command is also available on Windows 
  using the Windows System for Linux once `flake8` is installed in the 
  local Linux environment. While passing this test is not required, it is good practice and it help 
  improve code quality in `statsmodels`.
* Docstring additions must render correctly, including escapes and LaTeX.

</details>


Problem statement:
BUG: Passing BSplines arguments `degree` or `df` as integers raises TypeError
#### Passing BSplines parameters `degree` or `df` as integers raises TypeError

The BSplines docstring specifies that the `degree` and `df` parameters should be integers, but if either is an integer then a TypeError is raised

#### Code Sample

```python
import numpy as np
from statsmodels.gam.smooth_basis import BSplines
BSplines(np.linspace(0, 1, 100).reshape(-1, 1), 6, 3)
```

#### Error output

<details>

```python
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-f85aa4844041> in <module>
      1 import numpy as np
      2 from statsmodels.gam.smooth_basis import BSplines
----> 3 splines = BSplines(np.linspace(0, 1, 100).reshape(-1, 1), 6, 3)
      4 splines

\lib\site-packages\statsmodels\gam\smooth_basis.py in __init__(self, x, df, degree, include_intercept, constraints, variable_names, knot_kwds)
    911 
    912         super(BSplines, self).__init__(x, include_intercept=include_intercept,
--> 913                                        variable_names=variable_names)
    914 
    915     def _make_smoothers_list(self):

\lib\site-packages\statsmodels\gam\smooth_basis.py in __init__(self, x, variable_names, include_intercept, **kwargs)
    749             self.variable_names = variable_names
    750 
--> 751         self.smoothers = self._make_smoothers_list()
    752         self.basis = np.hstack(list(smoother.basis
    753                                for smoother in self.smoothers))

\lib\site-packages\statsmodels\gam\smooth_basis.py in _make_smoothers_list(self)
    919             uv_smoother = UnivariateBSplines(
    920                 self.x[:, v],
--> 921                 df=self.dfs[v], degree=self.degrees[v],
    922                 include_intercept=self.include_intercept[v],
    923                 constraints=self.constraints,

TypeError: 'int' object is not subscriptable
```
</details>

#### Expected Output

A `BSplines` instance to be created.

#### Output of ``import statsmodels.api as sm; sm.show_versions()``

<details>

INSTALLED VERSIONS
------------------
Python: 3.7.5.final.0

statsmodels
===========

Installed: 0.11.1

Required Dependencies
=====================

cython: 0.29.21
numpy: 1.18.5
scipy: 1.5.0
pandas: 0.25.3
    dateutil: 2.8.1
patsy: 0.5.1

Optional Dependencies
=====================

matplotlib: 3.2.2
    backend: module://ipykernel.pylab.backend_inline 
cvxopt: Not installed
joblib: 0.16.0

Developer Tools
================

IPython: 7.16.1
    jinja2: 2.11.2
sphinx: 3.1.2
    pygments: 2.6.1
pytest: 5.4.3
virtualenv: 16.7.8

</details>


BUG: AdditiveGamSmoother.transform fails for 1-D arrays
#### Describe the bug

Passing a 1-D numpy array, or a pandas DataFrame or Series as the `x` parameter to `BSplines` succeeds in returning a `BSplines` instance, but then fails to transform the same `x` when passed as the argument to `BSplines.transform`

#### Code Sample

```python
import numpy as np
import pandas as pd
from statsmodels.gam.smooth_basis import BSplines

x = np.linspace(0, 1, 100)
df = pd.DataFrame(dict(x1=x, x2=x))

splines = BSplines(x, [6], [3])
splines.transform(x)

df_splines = BSplines(df, [6, 6], [3, 3])
df_splines.transform(df)

series_splines = BSplines(df["x1"], [6], [3])
series_splines.transform(df["x1"])
```
- A 1-D ndarray raises `IndexError: too many indices for array`
- A DataFrame raises `TypeError: '(slice(None, None, None), 0)' is an invalid key`
- A Series raises `ValueError: Can only tuple-index with a MultiIndex`

#### Expected Output

Transformed arrays

#### Output of ``import statsmodels.api as sm; sm.show_versions()``

<details>
INSTALLED VERSIONS
------------------
Python: 3.7.5.final.0

statsmodels
===========

Installed: 0.11.1

Required Dependencies
=====================

cython: 0.29.21
numpy: 1.18.5
scipy: 1.5.0
pandas: 0.25.3
    dateutil: 2.8.1
patsy: 0.5.1

Optional Dependencies
=====================

matplotlib: 3.2.2
    backend: module://ipykernel.pylab.backend_inline 
cvxopt: Not installed
joblib: 0.16.0

Developer Tools
================

IPython: 7.16.1
    jinja2: 2.11.2
sphinx: 3.1.2
    pygments: 2.6.1
pytest: 5.4.3
virtualenv: 16.7.8

</details>

Hints from issue discussion:

Commit message: Fixed BSplines to match existing docs

- The docstring for `BSplines` appears to have been copied from that of
  `UnivariateCubicCyclicSplines`, as it specified the `df` and `degree`
  args to be integers, but in fact required them to be lists (raising a
  `TypeError: 'int' object is not subscriptable` if either was an
  integer)
  - Fixed docstring to specify `{list[int], int}` instead
  - Implemented support for integers by converting `df` and `degree` to
    lists if they aren't already

- Added `test_bsplines` parameterised test to test the creation of
  BSplines instances and the transformation of their original data

Patch to review:
diff --git a/statsmodels/gam/smooth_basis.py b/statsmodels/gam/smooth_basis.py
index 15338c29b79..fb5be52e78f 100644
--- a/statsmodels/gam/smooth_basis.py
+++ b/statsmodels/gam/smooth_basis.py
@@ -616,7 +616,7 @@ class UnivariateCubicCyclicSplines(UnivariateGamSmoother):
     x : ndarray, 1-D
         underlying explanatory variable for smooth terms.
     df : int
-        numer of basis functions or degrees of freedom
+        number of basis functions or degrees of freedom
     degree : int
         degree of the spline
     include_intercept : bool
@@ -792,6 +792,9 @@ def transform(self, x_new):
         basis : ndarray
             design matrix for the spline basis for given ``x_new``.
         """
+        x_new = np.asarray(x_new)
+        if x_new.ndim == 1:
+            x_new = x_new.reshape(-1, 1)
         exog = np.hstack(list(self.smoothers[i].transform(x_new[:, i])
                          for i in range(self.k_variables)))
         return exog
@@ -839,10 +842,13 @@ class BSplines(AdditiveGamSmoother):
         underlying explanatory variable for smooth terms.
         If 2-dimensional, then observations should be in rows and
         explanatory variables in columns.
-    df :  int
-        numer of basis functions or degrees of freedom
-    degree : int
-        degree of the spline
+    df :  {list[int], int}
+        number of basis functions or degrees of freedom; should be equal
+        in length to the number of columns of `x`; may be an integer if
+        `x` has one column or is 1-D.
+    degree : {list[int], int}
+        degree(s) of the spline; the same length and type rules apply as
+        to `df`
     include_intercept : bool
         If False, then the basis functions are transformed so that they
         do not include a constant. This avoids perfect collinearity if
@@ -907,8 +913,8 @@ class BSplines(AdditiveGamSmoother):
     """
     def __init__(self, x, df, degree, include_intercept=False,
                  constraints=None, variable_names=None, knot_kwds=None):
-        self.degrees = degree
-        self.dfs = df
+        self.degrees = [degree] if isinstance(degree, int) else degree
+        self.dfs = [df] if isinstance(df, int) else df
         self.knot_kwds = knot_kwds
         # TODO: move attaching constraints to super call
         self.constraints = constraints
diff --git a/statsmodels/gam/tests/test_smooth_basis.py b/statsmodels/gam/tests/test_smooth_basis.py
index f694d1d088c..5ba672a9142 100644
--- a/statsmodels/gam/tests/test_smooth_basis.py
+++ b/statsmodels/gam/tests/test_smooth_basis.py
@@ -6,10 +6,12 @@
 
 """
 
+import pytest
 import numpy as np
 from numpy.testing import assert_allclose, assert_equal
 from statsmodels.gam.smooth_basis import (UnivariatePolynomialSmoother,
-                                          PolynomialSmoother)
+                                          PolynomialSmoother,
+                                          BSplines)
 
 
 def test_univariate_polynomial_smoother():
@@ -27,3 +29,14 @@ def test_multivariate_polynomial_basis():
     for i, deg in enumerate(degrees):
         uv_basis = UnivariatePolynomialSmoother(x[:, i], degree=deg).basis
         assert_allclose(mps.smoothers[i].basis, uv_basis)
+
+@pytest.mark.parametrize(
+    "x, df, degree",
+    [
+        (np.linspace((0, 1), (1, 10), 100), [5, 6], [3, 5]),
+        (np.linspace(0, 1, 100).reshape(-1, 1), 6, 3),
+    ]
+)
+def test_bsplines(x, df, degree):
+    bspline = BSplines(x, df, degree)
+    bspline.transform(x)


Code review comments:
diff_hunk: @@ -792,6 +792,9 @@ def transform(self, x_new):
         basis : ndarray
             design matrix for the spline basis for given ``x_new``.
         """
+        x_new = np.asarray(x_new)
path: statsmodels/gam/smooth_basis.py
text: @user1:
You shouldn't case to array since x_new is expected to already be ndarray.

@author:
Thanks for the comments! I think I've implemented all the requested changes. On this point, I've removed the offending line, but `AdditiveGamSmoother` supports the use of pandas DataFrames and Series at creation, so would adding support for them to `transform` be considered an enhancement?

diff_hunk: @@ -839,10 +842,13 @@ class BSplines(AdditiveGamSmoother):
         underlying explanatory variable for smooth terms.
         If 2-dimensional, then observations should be in rows and
         explanatory variables in columns.
-    df :  int
-        numer of basis functions or degrees of freedom
-    degree : int
-        degree of the spline
+    df :  {list[int], int}
path: statsmodels/gam/smooth_basis.py
text: @user1:
{int, array_like[int]} is more precise since np.array(...,dtype=int) is a good input.

diff_hunk: @@ -907,8 +913,8 @@ class BSplines(AdditiveGamSmoother):
     """
     def __init__(self, x, df, degree, include_intercept=False,
                  constraints=None, variable_names=None, knot_kwds=None):
-        self.degrees = degree
-        self.dfs = df
+        self.degrees = [degree] if isinstance(degree, int) else degree
path: statsmodels/gam/smooth_basis.py
text: @user1:
This is where the upcasting to an int array should happen.



@user1:
Same for below.