-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
improve docs of broadcasting section #47529
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
base: master
Are you sure you want to change the base?
Conversation
A similar/equivalent work #44170 as a reference but this one seems more readable. 😢 |
doc/src/manual/arrays.md
Outdated
|
||
``` | ||
Broadcasting of a and B: | ||
fill expand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here (and above) you say "fill" and "expand". But fill
means something else. Perhaps clearer to use always the verbs which are commands: "reshape" and "repeat".
I also think the paragraph above could be much shorter. It need not read the whole table out in prose before showing the table. Just show the table, find one sentence to introduce it? Keep the table as close as possible to the examples above where we can see the results.
I'd also make c
not re-use the numbers from B
, so that the results are visually more distinct. Maybe it should even be c = [10 100 1000;;;]
so that the example also introduces dimensions beyond the 2nd, even if they are trivial.
The fact that the first two dimensions (of 3 or more) are printed as being a matrix is what fits well with the start-at-the-beginning alignment of broadcast dims.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think "fill" is less confusing because it is obvious from the context that it is used as a general English word here. Using "reshape" or "repeat" might be more confusing because functions with these names exist in Julia and the reader might misunderstand that reshape
or repeat
is called when broadcasting arrays.
The paragraph might look verbose but I think we should provide enough explanation to introduce a new concept. While the behavior of broadcasting is almost obvious to us, it is not always the case to others. I think this paragraph is important for readers to confirm their mental model in words.
I've changed c
to [10 100 1000]
. [10 100 1000;;;]
looks distracting to me because the syntax is rarely used.
@mcabbott I've refreshed the pull request entirely, reflecting your suggestions. Can you check it again? |
Any chance to get this merged? |
Triage differs to @mcabbott |
05f72ea
to
34c5f6c
Compare
Sorry I missed this!
I wonder a bit if the whole idea of virtual repeats can just be scrapped. We need not think of |
This improves the documentation of broadcasting, which proposes: