Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: fix moments method to support arrays and list #12197

Merged
merged 29 commits into from Aug 27, 2021
Merged
Changes from 9 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2f1beae
BUG: fix moments method to support arrays and list
tirthasheshpatel May 23, 2020
2f0a903
FIXUP: fix the moment method to perform operations correctly
tirthasheshpatel May 23, 2020
8359f4a
FIUP: fix the int32 error in moment method
tirthasheshpatel May 23, 2020
65e359a
FIXUP: fix int64 error in moment
tirthasheshpatel May 23, 2020
5391f86
FIXUP: fix failing tests
tirthasheshpatel May 24, 2020
5ab67bf
FIXUP: filter warnings in test_moment
tirthasheshpatel May 24, 2020
e3fd4ef
Merge remote-tracking branch 'upstream/master' into fix-moment
mdhaber Nov 26, 2020
06c5162
BUG: stats: remove some redundant computation
tirthasheshpatel Nov 27, 2020
b7206d4
BUG: stats: fix an inplace operation
tirthasheshpatel Nov 27, 2020
60c517a
BUG: stats: remove all redundant computations
tirthasheshpatel Nov 30, 2020
78e3fe9
BUG, TST: stats: add tests and resolve the bug
tirthasheshpatel Nov 30, 2020
8ca99eb
MAINT: stats: refactor moment function for array input
mdhaber Dec 1, 2020
3ef2889
Merge pull request #1 from mdhaber/fix-moment
tirthasheshpatel Dec 2, 2020
ec282dd
TST: stats: remove rtol=1e-20 and assert equality
tirthasheshpatel Dec 3, 2020
4989c3c
BUG: stats: fix the TypeError bug in _moment_from_stats
tirthasheshpatel Dec 4, 2020
559bd3d
MAINT: stats: merge remote tracking branch 'master' and resolve confl…
tirthasheshpatel Dec 15, 2020
3dbf027
TST: stats: change assert_equal to assert_allclose
tirthasheshpatel Dec 28, 2020
a6f4788
Merge remote-tracking branch 'upstream/master' into fix-moment
mdhaber May 26, 2021
cd90f3e
TST: stats: improve tests of rv_continuous.moment with array arguments
mdhaber May 26, 2021
112b588
MAINT: stats: use [()] instead of .item()
tirthasheshpatel May 27, 2021
43858ec
BUG: stats: call argsreduce for all non-none moments
tirthasheshpatel May 27, 2021
2cebce8
MAINT: stats: apply suggestions from code review
tirthasheshpatel Jun 27, 2021
7b69ef6
MAINT: stats: add a suggestion from code review
tirthasheshpatel Jul 5, 2021
a13a73b
TST, CI: stats: add hypothesis to CI and add tests
tirthasheshpatel Aug 3, 2021
acf4c1b
CI: stats: add hypothesis to macOS job
tirthasheshpatel Aug 3, 2021
5b10a45
Revert "CI: stats: add hypothesis to macOS job"
tirthasheshpatel Aug 27, 2021
35387ff
Revert "TST, CI: stats: add hypothesis to CI and add tests"
tirthasheshpatel Aug 27, 2021
d6a9b11
STY: stats: fix lint failures
tirthasheshpatel Aug 27, 2021
7ed600d
STY: stats: fix E501 (line too long) PEP-8 failure
tirthasheshpatel Aug 27, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 19 additions & 12 deletions scipy/stats/_distn_infrastructure.py
Expand Up @@ -1240,8 +1240,13 @@ def moment(self, n, *args, **kwds):

"""
args, loc, scale = self._parse_args(*args, **kwds)
if not (self._argcheck(*args) and (scale > 0)):
return nan
arrs = tuple(map(np.asarray, [*args, loc, scale]))
Copy link
Contributor

Choose a reason for hiding this comment

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

This first assignment appears to be unused?

Copy link
Member Author

Choose a reason for hiding this comment

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

*args? I think I did it because loc and scale parameters need to be of the same shape as inputs (that are present in args) otherwise the output isn't consistent with input shapes...

Copy link
Contributor

Choose a reason for hiding this comment

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

The first line is not needed.

arrs = tuple(map(np.asarray, [*args, loc, scale]))
arrs = tuple(map(np.atleast_1d, [*args, loc, scale]))

Doing this causes the return value to always be an array if I'm understanding the code correctly.

Copy link
Member

Choose a reason for hiding this comment

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

atleast_1d receives a list of arrays already, so the tuple(map(...)) dance can be avoided.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks for the review!

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you still need the arrs = tuple(map(np.asarray, [*args, loc, scale])) even?
arrs = np.broadcast_arrays(*(*args, loc, scale)) accepts inputs that are not already arrays.

arrs = np.broadcast_arrays(*arrs)
args = arrs[:-2]
loc, scale = arrs[-2], arrs[-1]
cond0 = (self._argcheck(*args) and scale > 0)
mdhaber marked this conversation as resolved.
Show resolved Hide resolved
cond1 = (loc != 0)
output = zeros(loc.shape, dtype='d')
if (floor(n) != n):
raise ValueError("Moment must be an integer.")
if (n < 0):
Expand All @@ -1257,16 +1262,18 @@ def moment(self, n, *args, **kwds):

# Convert to transformed X = L + S*Y
# E[X^n] = E[(L+S*Y)^n] = L^n sum(comb(n, k)*(S/L)^k E[Y^k], k=0...n)
if loc == 0:
return scale**n * val
else:
result = 0
fac = float(scale) / float(loc)
for k in range(n):
valk = _moment_from_stats(k, mu, mu2, g1, g2, self._munp, args)
result += comb(n, k, exact=True)*(fac**k) * valk
result += fac**n * val
return result * loc**n
mdhaber marked this conversation as resolved.
Show resolved Hide resolved
fac = _lazywhere(cond1, (scale, loc),
lambda scale, loc: scale / loc, self.badvalue)
for k in range(n):
valk = _moment_from_stats(k, mu, mu2, g1, g2, self._munp, args)
output = output + comb(n, k, exact=True)*fac**k * valk
output = output + fac**n * val
output = output * loc**n
output = np.where(cond1, output, scale**n * val)
output = np.where(cond0, output, self.badvalue)
if output.ndim == 0:
return output.item()
return output

def median(self, *args, **kwds):
"""
Expand Down