-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Check ndim of input arrays. (utils.check_arrays) #1597
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
Comments
The docstring is just a bit off: what it means is What would be the benefit of having several functions? |
It's not really necessary to separate things into separate functions, but I was thinking that one might want to check only certain subsets of (1,2,3, and 4). For example, some functions might not care if you send in a 1D or a 2D array, but they might still require that the first dimension be the same in both inputs. Another reason might be readability / code reuse. Again, not really necessary. As you said, I think the main point is to check for ndim. |
+1 for breaking |
why did you find the use complicated? |
It has a lot of options and the code is quite complex. It also doesn't do all the input validation that is required at |
.. but doesn't check for finite numbers and always enforces the same sparsity type... I think if we will split up the function, that will lead to code duplication and will make it harder to have consistent checks. Feel free to try, though ;) |
Hi I am new and trying to make a contribution. I would like to work on a PR for this issue, I think since the situations where ndim > 2 should be consistent across all estimators it should be the case that the check_arrays function raises an error along the lines of "max array dimension has been exceeded". Guidance will be much appreciated. |
Hi @hamsal, Indeed, you'd want to insert a check for |
@larsmans Thanks for the note to include a unit test. |
Closing as solved by #2958. |
As discussed in #1549, utils.check_arrays() does not check the shape or ndim of input arrays. However, the docstring suggests that it should: "Checks whether all objects in arrays have the same shape or length."
To me, I think what we need is to break check_arrays() into several sub-functions or maybe a class:
The text was updated successfully, but these errors were encountered: