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

Define Base.iterate(::Nothing) = nothing ? #29877

Closed
samoconnor opened this issue Nov 1, 2018 · 16 comments
Closed

Define Base.iterate(::Nothing) = nothing ? #29877

samoconnor opened this issue Nov 1, 2018 · 16 comments

Comments

@samoconnor
Copy link
Contributor

Would it make sense to define Base.iterate(::Nothing) = nothing ?

HTTP.jl has a generic interface that accepts anything iterable.
A user has reported method matching iterate(::Nothing) when they passed nothing.

It seems that eltype is already defined.

julia> eltype(nothing)
Any

But, HasEltype, HasShape and HasLength are not.

@KristofferC
Copy link
Sponsor Member

It seems that eltype is already defined.

Well,

julia> struct Gronk end

julia> eltype(Gronk())
Any

@JeffBezanson
Copy link
Sponsor Member

No, I don't think we should do this. Nothing is not a 0-element iterator, it's nothing.

eltype has a fallback method for all types. I tried hard to get rid of it before 1.0 but was not successful.

@samoconnor
Copy link
Contributor Author

I see a pattern of field::Union{Something,Nothing} being used in various places.
I guess there is a choice between:

  • A: defining harmless do-nothing methods for Nothing arguments, so that code that might be passed nothing just does nothing (maybe after many layers of generic-code calling other generic-code).
  • B: putting if x !== nothing where x is first accessed so that all the generic code after that can assume that it will always be passed Something.

If b is the prevalent pattern, then I guess my answer to the HTTP.jl user should be that they should not pass body= at all if they don't want to send a body (it is an optional argument), or if they really want to they can can pass "", or [] or HTTP.nobody

@JeffBezanson
Copy link
Sponsor Member

Yes B is the intended use. Unions with Nothing are like Option types; you're supposed to check before using the value. However it would be perfectly reasonable for HTTP to support body=nothing. It seems to me it could do that by checking instead of by making Nothing iterable.

@samoconnor
Copy link
Contributor Author

Currently we have this:

const nobody = UInt8[]

function request(method, url, h=Header[], b=nobody;
                 headers=h, body=b, query=nothing, kw...)::Response

Would you suggest that it is better to say:

function request(method, url, h=Header[], b=nothing;
                 headers=h, body=b, query=nothing, kw...)::Response

(and add the check for === nothing at the place that body sending is done)

@StefanKarpinski
Copy link
Sponsor Member

Does UInt8[] not behave the way you want it to?

@mbauman
Copy link
Sponsor Member

mbauman commented Nov 1, 2018

Duplicate of #26695.

@samoconnor
Copy link
Contributor Author

Does UInt8[] not behave the way you want it to?

UInt8[] behaves the way I want it to.
A HTTP.jl user asked: "why can't I pass body=nothing?".

@KristofferC
Copy link
Sponsor Member

Answer: "Because nothing isn't iterable."

@StefanKarpinski
Copy link
Sponsor Member

You can also do body === nothing && body = UInt8[] at the top of the function.

@quinnj
Copy link
Member

quinnj commented Nov 1, 2018

The real issue here is that there's no way for us to define request(url, headers, body::Iterable) = .... It's not really that helpful to say "because nothing isn't iterable", because there are an infinite number of additional types that aren't iterable that would also cause equal confusion for users if they tried to pass as the body. Probably the best course of action for the moment would to include an argument validation check like Base.isiterable(body) || throw(ArgumentError("body argument must be iterable"))

@KristofferC
Copy link
Sponsor Member

And the default error message is not good because it looks like that it might as well have been a bug in HTTP?

@samoconnor
Copy link
Contributor Author

Answer: "Because nothing isn't iterable."

The doc says:

body can take a number of forms:

  • a String, a Vector{UInt8} or any T accepted by write(::IO, ::T)
  • a collection of String or AbstractVector{UInt8} or IO streams
    or items of any type T accepted by write(::IO, ::T...)
  • a readable IO stream or any IO-like type T for which
    eof(T) and readavailable(T) are defined.

The remaining question is simply: (to conform to the principle of least surprise for users) should the interface accept body=nothing?

You can also do body === nothing && body = UInt8[] at the top of the function.
Sure, if we accept body=nothing, implementing it is no problem.

The reason for filing this issue was that if Julia's Nothing was iterable, the problem wouldn't have come up. I'm not advocating for Nothing to be iterable.

@samoconnor
Copy link
Contributor Author

And the default error message is not good because it looks like that it might as well have been a bug in HTTP?

No, I think the error message is fine as it is.
In a system that uses duck-typing rather than formal protocols, users should be accustomed to the fact that they'll get method not found errors from deep inside a generic library function if they thing they passed in is not as duck-like as they'd hoped.

If we put too many restrictive preconditions on things we'll loose the benefits of being able to keep things loosely coupled by implementing only as much duck-ness as is needed in a particular situation.

I recall some particularly painful experiences in other languages where some API wants a FooInterfaceType thing and I know my use of it is only going to call two of the 50 methods in that interface, and I just want to try it out quickly, but I have to fill in all 50 methods before the complier will let me.

@JeffBezanson
Copy link
Sponsor Member

The remaining question is simply: (to conform to the principle of least surprise for users) should the interface accept body=nothing?

This is a very good question, and a bit tricky. Allowing body=nothing seems ok to me, but the cost is always that it hides potential bugs like body=print("hello"). Could go either way I think.

@samoconnor
Copy link
Contributor Author

I'm happy to stick with the current API where you can pass [] or "" or HTTP.nobody (or just not supply a body= argument at all).
It is great to have nothing as an alternative to using -1 to mean "we don't have a real value for this number". But if an API says "you can optionally supply some bytes, or a string, or an IOStream" it seems fine to just use [] to mean "i don't want to send any bytes".

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

No branches or pull requests

6 participants