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

Improve message error in broadcast: "arrays could not be broadcast to a common size" #32866 #32867

Merged
merged 3 commits into from
Aug 13, 2019

Conversation

dmolina
Copy link
Contributor

@dmolina dmolina commented Aug 11, 2019

Following suggestion from ##30438, I have update messages indicating the distance. Now the error message is:

"arrays could not be broadcast to a common size; computed a dimension with lengths $(length(a)) and $(length(b))"

I consider that good error messages is very important for the user-friendliness. I have suffered the previous error message, and in my opinion it should be changed.

In #30438 it was suggested to use directly "$a", not "$(length(a))", but in that case it always said Base.OneTo(X) and with length says directly "X", when the data is Integer, length is not used but only the direct value "$a".

@ararslan ararslan requested a review from mbauman August 11, 2019 19:27
@ararslan ararslan added the broadcast Applying a function over a collection label Aug 11, 2019
@mbauman mbauman added the error handling Handling of exceptions by Julia or the user label Aug 12, 2019
@mbauman
Copy link
Member

mbauman commented Aug 12, 2019

This is great, thank you! I know you're just using the wording I myself suggested, but could you change computed to simply got? I don't think there's any recomputation involved here — we're just checking the arguments that broadcast received and are reporting back.

@dmolina
Copy link
Contributor Author

dmolina commented Aug 13, 2019

Thanks you. I have updated the error message as you suggested, changing 'calculated' to 'got'.

@StefanKarpinski StefanKarpinski added the forget me not PRs that one wants to make sure aren't forgotten label Aug 13, 2019
@mbauman mbauman merged commit 4537d78 into JuliaLang:master Aug 13, 2019
@tkoolen
Copy link
Contributor

tkoolen commented Aug 14, 2019

How does this affect performance, in light of #29688?

@simeonschaub simeonschaub removed the forget me not PRs that one wants to make sure aren't forgotten label May 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection error handling Handling of exceptions by Julia or the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants