Julia: Julia does not dispatch correctly?

[essenciary]

Hi,

I find this confusing. Is this correct?

julia> b = "bar"
"bar"

julia> Base.get(b)
ERROR: MethodError: `get` has no method matching get(::ASCIIString)

julia> using Requests

julia> Base.get(b)
ERROR: Unsupported scheme "bar"
 in open_stream at /Users/adrian/.julia/v0.4/Requests/src/streaming.jl:179
 in do_stream_request at /Users/adrian/.julia/v0.4/Requests/src/Requests.jl:346
 in do_request at /Users/adrian/.julia/v0.4/Requests/src/Requests.jl:286
 in get at /Users/adrian/.julia/v0.4/Requests/src/Requests.jl:407
 in get at /Users/adrian/.julia/v0.4/Requests/src/Requests.jl:406

The second time, Julia dispatches to Request.get though I specifically call Base.get?

[JeffBezanson]

Looks like Requests is adding a method to Base.get.

[ararslan]

Oddly enough, the Requests package doesn't appear to be modifying Base.get explicitly (at least on the current master). It defines its own get function and doesn't export it. What version of the package are you using? Could you Pkg.update() and try again?

[essenciary]

Currently at

- Requests                      0.3.7

Updating...

[essenciary]

Same version after update.

 - Requests                      0.3.7

In case this helps:

julia> versioninfo()
Julia Version 0.4.5
Commit 2ac304d* (2016-03-18 00:58 UTC)
Platform Info:
  System: Darwin (x86_64-apple-darwin15.4.0)
  CPU: Intel(R) Core(TM) i7-3740QM CPU @ 2.70GHz
  WORD_SIZE: 64
  BLAS: libopenblas (DYNAMIC_ARCH NO_AFFINITY Sandybridge)
  LAPACK: libopenblas
  LIBM: libopenlibm
  LLVM: libLLVM-3.3

[ararslan]

Maybe @malmaud could provide some insight here as to what Requests is actually doing.

[malmaud]

Requests does add to Base.get. It has an import Base: get statement somewhere.

[ararslan]

Whoops, my bad.

[ararslan]

Looks like Julia's dispatch is just fine then. :smile:

[essenciary]

OK, so it is caused by Requests. Thanks - I should've looked into Requests but I haven't considered this, my apologies.

Sorry, but I don't understand the benefit of this. Why would it be added to Base?

I accidentally passed a string to Base.get (some function in some edge case accidentally returned the string instead of the Nullable I expected) and then I spent an hour debugging because the error was bubbling somewhere else.

It's very unintuitive - I might be wrong but I expected Base.get#1 (arity 1) to provide functionality related to Nullables. And in the case of the string maybe throw an error that the argument is not a Nullable type. Well, which actually does when Requests in not imported...

For me this breaks the principle of least surprise. I understand it's an architectural decision within Requests, but if Julia could handle / guard against these issues, at least with a warning, it would probably save a lot of time and frustrations down the line.

[essenciary]

Yes, nothing wrong with the dispatch then :)
Closing this, thanks for your help, sorry for the false alarm.

[ararslan]

You could open an issue on Requests and suggest not exporting Base.get, which means all GET requests would need to be performed using Requests.get and Base.get would be left unaltered (and could be used as get without surprise).

[essenciary]

I will, definitely 👍

[tkelman]

Extending Base methods for types that aren't owned by a package is really not kosher, Requests shouldn't be doing that and we should find a way to scan for when that happens.

[essenciary]

It would be great if the compiler could check, especially when we're talking about methods with one argument accepting very common built-in types, like ASCIIString in this case. I don't see a problem in the case of a custom type, like say a URI type of some sorts. Indeed, types owned by the package itself.

Alternatively, the compiler could maybe generate / reserve the methods in Base for basic built-in types (strings, numbers, chars, etc) even if they are not implemented, and simply throw a useful error? I remember seeing this pattern somewhere (maybe Elixir, not sure).

Otherwise / until then at least explicitly recommend against this in the docs?

[malmaud]

Requests had been overloading get in this bad way since before I started maintaining it, and I think unfortunately the deprecation headaches of changing it now would far outweigh the gains (it's the most popular method in a pretty widely-used package). But at any rate, open an issue on Requests if you want to discuss further.

[tkelman]

We do need to find the right way to smoothly unexport a method without having to go through a renaming transition. Deprecate the unqualified usage with a warning so users have adequate time to fix it before you remove the export. It might be possible already with the right clever application of @deprecate_binding, not sure.