I agree with @Tamas_Papp that anticipating every possible user expectation in the warnings becomes too much trouble at some point. However, for the basic arithmetic operations (+,-,*,/,^) and functions (sqrt, exp, log) on elementary Number and AbstractArray types, I think it would be reasonable to put some extra effort into the warnings. These are some of the first situations where new users will encounter differences from Numpy/R/Matlab, and it is an opportunity to teach them the Julia semantics.
7 Likes
I do know this. I am familiar with software development. Yet, this is the kind of method that demands least amount of work possible. Both eachcol and eachrow need more maintenance than this, but exist.
I cannot see a problem if this is done consistently and with a good reasoning (i.e., the maintenance cost is expected to be worth the help to newcomers). Moreover, if there is a flood of requests of this kind that have no merit, well, they will be denied, I trust the ability of JuliaLang developers to say no.
I think I was not clear enough. The bug they find is the wrong use of eachrow when they wrongly assume it returns a row. So the bug is in their code. I think the existence of eachcol (in its current fashion) is maybe a design mistake, but I would not call it a bug as it seems very clear now that it was intended to be this way.
Yes. However, I find that, in this specific context, of an user trying to multiply two unidimensional arrays the changed error message will make the problem clear to the user in 90%+ of the cases, while I can hardly see a situation where the old error message would be better.
Never said it was not, and even suggested that eachrow could be clearer in its docstring about how it does not return a row from a matrix (or even change the term view to view to an Array{Any, 1}). This does not preclude creating this error message in this specific case.
@stevengj We agree. My idea was never to do this to every possible mistake involving the standard library that a programmer can do. Monday or Tuesday I will make a PR if none of my lab colleagues want to do it themselves.
1 Like
Another issue is that the docstring of eachrow is simply
Create a generator that iterates over the first dimension of vector or matrix A, returning the rows as views.
Reading this, I would expect eachrow to return a 1xN matrix view. This is also heavily implied by the function name.
Edit: Actually, the function name is fine, but combined with the docstring, I find it confusing.
3 Likes
Made a PR that clarifies the docstrings. I think that this resolves the issue.
5 Likes