Skip to content

many types should define broadcastable(o) = Ref(o) #44093

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Nov 2, 2022
Merged

many types should define broadcastable(o) = Ref(o) #44093

merged 5 commits into from
Nov 2, 2022

Conversation

@stevengj
Copy link
Member

@stevengj stevengj commented Feb 9, 2022

This is an important point because it is the most common case for user-defined types, but I noticed that it was a bit buried.

@stevengj stevengj added docs This change adds or pertains to documentation broadcast Applying a function over a collection labels Feb 9, 2022
fredrikekre
fredrikekre reviewed Feb 10, 2022
View reviewed changes
timholy
timholy reviewed Feb 10, 2022
View reviewed changes
@stevengj stevengj changed the title most types should define broadcastable(o) = Ref(o) many types should define broadcastable(o) = Ref(o) Feb 10, 2022
vtjnash
vtjnash reviewed Feb 10, 2022
View reviewed changes
@stevengj
Copy link
Member Author

stevengj commented Mar 31, 2022

Rebased. Okay to merge?

@vtjnash vtjnash added the triage This should be discussed on a triage call label Apr 4, 2022
@vtjnash
Copy link
Member

vtjnash commented Apr 4, 2022

I thought the past recommendation was away from this behavior being automatic, so perhaps triage should be given a chance to comment on the text?

@JeffBezanson
Copy link
Member

JeffBezanson commented Apr 14, 2022

Right, in other words we wanted to encourage wrapping values at the call site rather than adding methods for "every" type to automatically wrap them.

JeffBezanson
JeffBezanson reviewed Apr 14, 2022
View reviewed changes
@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Apr 14, 2022
@stevengj
Copy link
Member Author

stevengj commented Apr 15, 2022

CI failures seem to be unrelated cancellations/timeouts.

@stevengj
Copy link
Member Author

stevengj commented Oct 19, 2022 •
edited
Loading

Rebased. Since I addressed the triage suggestions, can this be merged?

@stevengj
Copy link
Member Author

stevengj commented Oct 22, 2022

Unrelated CI failures (ProcessExitedException in the Profile test due to some TLS authentication problem?), it looks like.

@KristofferC KristofferC merged commit e9ca677 into master Nov 2, 2022
@KristofferC KristofferC deleted the stevengj-patch-2 branch November 2, 2022 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vtjnash vtjnash vtjnash left review comments

@JeffBezanson JeffBezanson JeffBezanson left review comments

@timholy timholy timholy left review comments

@fredrikekre fredrikekre fredrikekre left review comments

Assignees

No one assigned

Labels

broadcast Applying a function over a collection docs This change adds or pertains to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@stevengj @vtjnash @JeffBezanson @timholy @fredrikekre @KristofferC