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

@pradhyumna85 hotfix to solve ValueError("Data must be 1-dimensional.") in boxcox, in case of non array input x and some lambda. #13305

Closed
wants to merge 2 commits into from

Conversation

pradhyumna85
Copy link

@pradhyumna85 pradhyumna85 commented Dec 30, 2020

What does this implement/fix?

ValueError("Data must be 1-dimensional.") in boxcox (scipy.stats.boxcox), in case of non array input x and some lmbda.

Hotfix to solve ValueError("Data must be 1-dimensional.") when only single value x (not array) is given with some value of lmbda.
hotfix to solve ValueError("Data must be 1-dimensional.") in boxcox
@mdhaber
Copy link
Contributor

mdhaber commented Dec 30, 2020

Is there a reference issue that explains the problem this is supposed to fix?
Do you have example code that causes the problem in the master branch? That code should be added in the form of a unit test.

@WarrenWeckesser
Copy link
Member

I think @pradhyumna85 expects this (for example) to work without raising an exception:

In [3]: boxcox(3.5, lmbda=2)                                                                                     
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-4429b9c008d5> in <module>
----> 1 boxcox(3.5, lmbda=2)

~/mc37allpip/lib/python3.7/site-packages/scipy-1.7.0.dev0+d293adb-py3.7-macosx-10.9-x86_64.egg/scipy/stats/morestats.py in boxcox(x, lmbda, alpha)
   1032     x = np.asarray(x)
   1033     if x.ndim != 1:
-> 1034         raise ValueError("Data must be 1-dimensional.")
   1035 
   1036     if x.size == 0:

ValueError: Data must be 1-dimensional.

It makes sense, because when lmbda is given, the function is just a thin wrapper around scipy.special.boxcox. We could simplify this PR by making the first two lines of the function

    if lmbda is not None:
        return special.boxcox(x, lmbda)

That is, move that if statement up from a few lines down. If we're just going to pass the values on to special.boxcox, the parameter validations aren't necessary.

This would change the behavior in a few cases. If any values in x are 0 or negative, stats.boxcox raises an exception. special.boxcox returns nan for those inputs. Also, special.boxcox is a ufunc and will broadcast its arguments, so both x and lmbda can have any shape, as long as the shapes are broadcastable.

If we did this, the docstring would need to be updated to explain that the requirement that x be 1-d and not constant applies only when lmbda is None. Also, no matter what we decide to do, special.boxcox should be added to the "See Also" section of the stats.boxcox docstring.

@pradhyumna85
Copy link
Author

I think @pradhyumna85 expects this (for example) to work without raising an exception:

In [3]: boxcox(3.5, lmbda=2)                                                                                     
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-4429b9c008d5> in <module>
----> 1 boxcox(3.5, lmbda=2)

~/mc37allpip/lib/python3.7/site-packages/scipy-1.7.0.dev0+d293adb-py3.7-macosx-10.9-x86_64.egg/scipy/stats/morestats.py in boxcox(x, lmbda, alpha)
   1032     x = np.asarray(x)
   1033     if x.ndim != 1:
-> 1034         raise ValueError("Data must be 1-dimensional.")
   1035 
   1036     if x.size == 0:

ValueError: Data must be 1-dimensional.

It makes sense, because when lmbda is given, the function is just a thin wrapper around scipy.special.boxcox. We could simplify this PR by making the first two lines of the function

    if lmbda is not None:
        return special.boxcox(x, lmbda)

That is, move that if statement up from a few lines down. If we're just going to pass the values on to special.boxcox, the parameter validations aren't necessary.

This would change the behavior in a few cases. If any values in x are 0 or negative, stats.boxcox raises an exception. special.boxcox returns nan for those inputs. Also, special.boxcox is a ufunc and will broadcast its arguments, so both x and lmbda can have any shape, as long as the shapes are broadcastable.

If we did this, the docstring would need to be updated to explain that the requirement that x be 1-d and not constant applies only when lmbda is None. Also, no matter what we decide to do, special.boxcox should be added to the "See Also" section of the stats.boxcox docstring.

@WarrenWeckesser Exactly what I meant. As it make sense to boxcox a single value if lmbda is given.

@rgommers rgommers added maintenance Items related to regular maintenance tasks scipy.stats labels Jan 28, 2021
@mdhaber
Copy link
Contributor

mdhaber commented Feb 28, 2022

Thanks @pradhyumna85! I think this was just fixed in gh-12225.

@mdhaber mdhaber closed this Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants