Skip to content

Commit

Permalink
Merge pull request #4951 from ev-br/distn_fit
Browse files Browse the repository at this point in the history
MAINT: do not ignore invalid kwargs in distributions fit method
  • Loading branch information
rgommers committed Jun 16, 2015
2 parents 8b53149 + d3a621f commit 6a4847e
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 15 deletions.
9 changes: 6 additions & 3 deletions scipy/stats/_continuous_distns.py
Expand Up @@ -419,8 +419,10 @@ def fit(self, data, *args, **kwds):
# Override rv_continuous.fit, so we can more efficiently handle the
# case where floc and fscale are given.

f0 = kwds.get('f0', None)
f1 = kwds.get('f1', None)
f0 = (kwds.get('f0', None) or kwds.get('fa', None) or
kwds.get('fix_a', None))
f1 = (kwds.get('f1', None) or kwds.get('fb', None) or
kwds.get('fix_b', None))
floc = kwds.get('floc', None)
fscale = kwds.get('fscale', None)

Expand Down Expand Up @@ -1885,7 +1887,8 @@ def _fitstart(self, data):

@inherit_docstring_from(rv_continuous)
def fit(self, data, *args, **kwds):
f0 = kwds.get('f0', None)
f0 = (kwds.get('f0', None) or kwds.get('fa', None) or
kwds.get('fix_a', None))
floc = kwds.get('floc', None)
fscale = kwds.get('fscale', None)

Expand Down
30 changes: 18 additions & 12 deletions scipy/stats/_distn_infrastructure.py
Expand Up @@ -1967,15 +1967,15 @@ def _reduce_func(self, args, kwds):
# shapes='a, b'. To fix `a`, can specify either `f1` or `fa`.
# Convert the latter into the former.
if self.shapes:
fshapes = ['f%s' % s for s in self.shapes.replace(',', ' ').split()]
for j, fs in enumerate(fshapes):
if fs in kwds:
shapes = self.shapes.replace(',', ' ').split()
for j, s in enumerate(shapes):
val = kwds.pop('f' + s, None) or kwds.pop('fix_' + s, None)
if val is not None:
key = 'f%d' % j
if key in kwds:
raise ValueError("Cannot specify both %s and %s" %
(fs, key))
raise ValueError("Duplicate entry for %s." % key)
else:
kwds.update({key: kwds[fs]})
kwds[key] = val

args = list(args)
Nargs = len(args)
Expand All @@ -1985,7 +1985,7 @@ def _reduce_func(self, args, kwds):
for n, key in enumerate(names):
if key in kwds:
fixedn.append(n)
args[n] = kwds[key]
args[n] = kwds.pop(key)
else:
x0.append(args[n])

Expand Down Expand Up @@ -2043,8 +2043,9 @@ def fit(self, data, *args, **kwds):
- f0...fn : hold respective shape parameters fixed.
Alternatively, shape parameters to fix can be specified by name.
For example, if ``self.shapes == "a, b"``, ``fa`` is equivalent to
``f0`` and ``fb`` is equivalent to ``f1``.
For example, if ``self.shapes == "a, b"``, ``fa``and ``fix_a``
are equivalent to ``f0``, and ``fb`` and ``fix_b`` are
equivalent to ``f1``.
- floc : hold location parameter fixed to specified value.
Expand Down Expand Up @@ -2110,12 +2111,12 @@ def fit(self, data, *args, **kwds):
# get distribution specific starting locations
start = self._fitstart(data)
args += start[Narg:-2]
loc = kwds.get('loc', start[-2])
scale = kwds.get('scale', start[-1])
loc = kwds.pop('loc', start[-2])
scale = kwds.pop('scale', start[-1])
args += (loc, scale)
x0, func, restore, args = self._reduce_func(args, kwds)

optimizer = kwds.get('optimizer', optimize.fmin)
optimizer = kwds.pop('optimizer', optimize.fmin)
# convert string to function in scipy.optimize
if not callable(optimizer) and isinstance(optimizer, string_types):
if not optimizer.startswith('fmin_'):
Expand All @@ -2126,6 +2127,11 @@ def fit(self, data, *args, **kwds):
optimizer = getattr(optimize, optimizer)
except AttributeError:
raise ValueError("%s is not a valid optimizer" % optimizer)

# by now kwds must be empty, since everybody took what they needed
if kwds:
raise TypeError("Unknown arguments: %s." % kwds)

vals = optimizer(func, x0, args=(ravel(data),), disp=0)
if restore is not None:
vals = restore(args, vals)
Expand Down
25 changes: 25 additions & 0 deletions scipy/stats/tests/test_distributions.py
Expand Up @@ -1249,17 +1249,42 @@ def test_fshapes(self):
res_2 = stats.beta.fit(x, fa=3.)
assert_allclose(res_1, res_2, atol=1e-12, rtol=1e-12)

res_2 = stats.beta.fit(x, fix_a=3.)
assert_allclose(res_1, res_2, atol=1e-12, rtol=1e-12)

res_3 = stats.beta.fit(x, f1=4.)
res_4 = stats.beta.fit(x, fb=4.)
assert_allclose(res_3, res_4, atol=1e-12, rtol=1e-12)

res_4 = stats.beta.fit(x, fix_b=4.)
assert_allclose(res_3, res_4, atol=1e-12, rtol=1e-12)

# cannot specify both positional and named args at the same time
assert_raises(ValueError, stats.beta.fit, x, fa=1, f0=2)

# check that attempting to fix all parameters raises a ValueError
assert_raises(ValueError, stats.beta.fit, x, fa=0, f1=1,
floc=2, fscale=3)

# check that specifying floc, fscale and fshapes works for
# beta and gamma which override the generic fit method
res_5 = stats.beta.fit(x, fa=3., floc=0, fscale=1)
aa, bb, ll, ss = res_5
assert_equal([aa, ll, ss], [3., 0, 1])

# gamma distribution
a = 3.
data = stats.gamma.rvs(a, size=100)
aa, ll, ss = stats.gamma.fit(data, fa=a)
assert_equal(aa, a)

def test_extra_params(self):
# unknown parameters should raise rather than be silently ignored
dist = stats.exponnorm
data = dist.rvs(K=2, size=100)
dct = dict(enikibeniki=-101)
assert_raises(TypeError, dist.fit, data, **dct)


class TestFrozen(TestCase):
# Test that a frozen distribution gives the same results as the original object.
Expand Down

0 comments on commit 6a4847e

Please sign in to comment.