Skip to content
This repository was archived by the owner on May 4, 2019. It is now read-only.

Fix broadcast() #153

Closed
nalimilan opened this issue Oct 4, 2016 · 4 comments
Closed

Fix broadcast() #153

nalimilan opened this issue Oct 4, 2016 · 4 comments

Comments

@nalimilan
Copy link
Member

We really need to fix this:

julia> isnull.(NullableArray(1:3))
ERROR: MethodError: no method matching broadcast_shape(::NullableArrays.NullableArray{Int64,1})
Closest candidates are:
  broadcast_shape(::Tuple) at broadcast.jl:53
  broadcast_shape(::Tuple, ::Tuple, ::Tuple...) at broadcast.jl:54
 in broadcast(::Function, ::NullableArrays.NullableArray{Int64,1}) at ./<missing>:0
@davidagold
Copy link
Contributor

We do. I'm pretty tired of trying to track changes to broadcast in Base. I vote that we just stop hosting our own broadcast implementation entirely, and rely on the AbstractArray interface. AFAICT, the only things we'd lose are

  1. some performance since we're no longer operating directly on the :values and :isnull fields of the argument NullableArrays
  2. the lift keyword argument

I'm fine with both of those for now.

@nalimilan
Copy link
Member Author

I don't have the experience of working with that code myself, so I can't really tell what's best. But indeed if we can't fix it soon, better rely on the Base implementation.

Also, regarding 1, I would have thought the compiler would be able to optimize getindex away. But I haven't checked (and I think you did). Regarding 2, your lift function could be a good replacement (and a more general one).

@davidagold
Copy link
Contributor

I haven't checked. I need to learn how to read lowered code. And yes, I intend the higher-order lift function to be a solution for this case, since writing broadcast(f, X, lift=true) is not any more convenient than writing broadcast(lift(f), X).

@nalimilan
Copy link
Member Author

Turns out these functions were simply renamed (not sure that was a good idea...). See #164.

So we can wait until we're more confident about removing the special methods for broadcast.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants