Repository: JuliaPy/PyCall.jl (julia)

Issue:
showable error on certain objects
On PyCall v1.91.4, Julia 1.5,

```julia
julia> using Conda, PyCall

julia> Conda.add("scikit-learn")   # sorry that it's such a big package

julia> showable(MIME"text/html"(), pyimport("sklearn.linear_model").LogisticRegression)
ERROR: PyError ($(Expr(:escape, :(ccall(#= /Users/cedric/.julia/packages/PyCall/zqDXB/src/pyfncall.jl:43 =# @pysym(:PyObject_Call), PyPtr, (PyPtr, PyPtr, PyPtr), o, pyargsptr, kw))))) <class 'TypeError'>
TypeError("'property' object is not callable")

Stacktrace:
 [1] pyerr_check at /Users/cedric/.julia/packages/PyCall/zqDXB/src/exception.jl:60 [inlined]
 [2] pyerr_check at /Users/cedric/.julia/packages/PyCall/zqDXB/src/exception.jl:64 [inlined]
 [3] _handle_error(::String) at /Users/cedric/.julia/packages/PyCall/zqDXB/src/exception.jl:81
 [4] macro expansion at /Users/cedric/.julia/packages/PyCall/zqDXB/src/exception.jl:95 [inlined]
 [5] #110 at /Users/cedric/.julia/packages/PyCall/zqDXB/src/pyfncall.jl:43 [inlined]
 [6] disable_sigint at ./c.jl:446 [inlined]
 [7] __pycall! at /Users/cedric/.julia/packages/PyCall/zqDXB/src/pyfncall.jl:42 [inlined]
 [8] _pycall!(::PyObject, ::PyObject, ::Tuple{}, ::Int64, ::Ptr{Nothing}) at /Users/cedric/.julia/packages/PyCall/zqDXB/src/pyfncall.jl:29
 [9] _pycall! at /Users/cedric/.julia/packages/PyCall/zqDXB/src/pyfncall.jl:11 [inlined]
 [10] #pycall#115 at /Users/cedric/.julia/packages/PyCall/zqDXB/src/pyfncall.jl:80 [inlined]
 [11] pycall at /Users/cedric/.julia/packages/PyCall/zqDXB/src/pyfncall.jl:80 [inlined]
 [12] showable(::MIME{Symbol("text/html")}, ::PyObject) at /Users/cedric/.julia/packages/PyCall/zqDXB/src/PyCall.jl:894
 [13] top-level scope at REPL[25]:1
```

For context, https://github.com/fonsp/Pluto.jl/issues/412

It might be a scikit-learn issue, but the error message is pretty cryptic.

Pull request description:
fix #816

An MWE

```julia
py"""
class Issue816(object):
    def _repr_html_(self):
        return "<h1>Issue816</h1>"
"""

showable("text/html", py"Issue816()")  # => true
showable("text/html", py"Issue816")  # => throws
```

```
ERROR: PyError ($(Expr(:escape, :(ccall(#= /home/takafumi/.julia/dev/PyCall/src/pyfncall.jl:43 =# @pysym(:PyObject_Call), PyPtr, (PyPtr, PyPtr, PyPtr), o, pyargsptr, kw))))) <class 'TypeError'>
TypeError("_repr_html_() missing 1 required positional argument: 'self'")

Stacktrace:
 [1] pyerr_check at /home/takafumi/.julia/dev/PyCall/src/exception.jl:62 [inlined]
 [2] pyerr_check at /home/takafumi/.julia/dev/PyCall/src/exception.jl:66 [inlined]
 [3] _handle_error(::String) at /home/takafumi/.julia/dev/PyCall/src/exception.jl:83
 [4] macro expansion at /home/takafumi/.julia/dev/PyCall/src/exception.jl:97 [inlined]
 [5] #110 at /home/takafumi/.julia/dev/PyCall/src/pyfncall.jl:43 [inlined]
 [6] disable_sigint at ./c.jl:446 [inlined]
 [7] __pycall! at /home/takafumi/.julia/dev/PyCall/src/pyfncall.jl:42 [inlined]
 [8] _pycall!(::PyObject, ::PyObject, ::Tuple{}, ::Int64, ::Ptr{Nothing}) at /home/takafumi/.julia/dev/PyCall/src/pyfncall.jl:29
 [9] _pycall! at /home/takafumi/.julia/dev/PyCall/src/pyfncall.jl:11 [inlined]
 [10] #pycall#115 at /home/takafumi/.julia/dev/PyCall/src/pyfncall.jl:80 [inlined]
 [11] pycall at /home/takafumi/.julia/dev/PyCall/src/pyfncall.jl:80 [inlined]
 [12] showable(::MIME{Symbol("text/html")}, ::PyObject) at /home/takafumi/.julia/dev/PyCall/src/PyCall.jl:901
 [13] showable(::String, ::Any) at ./multimedia.jl:77
 [14] top-level scope at /home/takafumi/.julia/dev/PyCall/src/pyeval.jl:232
```

The error is due to invoking `Issue816._repr_html_()`. A simple solution seems to just return `false` from `showable("text/html", o)` when `o` is a python type. Although it's probably technically possible to implement `_repr_html_` on classes as well, it's very likely virtually nobody uses it.

Patch:
diff --git a/src/PyCall.jl b/src/PyCall.jl
index c260bbd4..fe0f2ae9 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -311,6 +311,11 @@ end
 getproperty(o::PyObject, s::AbstractString) = __getproperty(o, s)
 getproperty(o::PyObject, s::Symbol) = convert(PyAny, __getproperty(o, s))
 
+function trygetproperty(o::PyObject, s::AbstractString, d)
+    p = _getproperty(o, s)
+    return p == C_NULL ? d : PyObject(p)
+end
+
 propertynames(o::PyObject) = ispynull(o) ? Symbol[] : map(x->Symbol(first(x)), PyIterator{PyObject}(pycall(inspect."getmembers", PyObject, o)))
 
 # avoiding method ambiguity
@@ -890,20 +895,64 @@ for (mime, method) in ((MIME"text/html", "_repr_html_"),
     T = istextmime(mime()) ? AbstractString : Vector{UInt8}
     @eval begin
         function show(io::IO, mime::$mime, o::PyObject)
-            if !ispynull(o) && hasproperty(o, $method)
-                r = pycall(o.$method, PyObject)
+            m = get_real_method(o, $method)
+            if m !== nothing
+                r = pycall(m, PyObject)
                 !(r ≛ pynothing[]) && return write(io, convert($T, r))
             end
             throw(MethodError(show, (io, mime, o)))
         end
-        Base.showable(::$mime, o::PyObject) =
-            !ispynull(o) && hasproperty(o, $method) && let meth = o.$method
-                !(meth ≛ pynothing[]) &&
-                !(pycall(meth, PyObject) ≛ pynothing[])
-            end
+        function Base.showable(::$mime, o::PyObject)
+            m = get_real_method(o, $method)
+            m === nothing && return false
+            return !(pycall(m, PyObject) ≛ pynothing[])
+        end
     end
 end
 
+"""
+    get_real_method(obj::PyObject, name::String) -> method::Union{PyObject,Nothing}
+
+This is a port of `IPython.utils.dir2.get_real_method`:
+https://github.com/ipython/ipython/blob/7.9.0/IPython/utils/dir2.py
+
+For Python 2-era
+https://github.com/ipython/ipython/blob/5.9.0/IPython/utils/dir2.py
+"""
+function get_real_method(obj, name)
+    ispynull(obj) && return nothing
+    @static if pyversion_build < v"3"
+        pyisinstance(obj, @pyglobalobj :PyType_Type) && return nothing
+    end
+
+    canary = try
+        trygetproperty(obj, "_ipython_canary_method_should_not_exist_", nothing)
+    catch
+        nothing
+    end
+
+    # It claimed to have an attribute it should never have
+    canary === nothing || return nothing
+
+    m = try
+        trygetproperty(obj, name, nothing)
+    catch
+        nothing
+    end
+    m === nothing && return nothing
+
+    if (
+        pyisinstance(obj, @pyglobalobj :PyType_Type) &&
+        !pyisinstance(m, @pyglobalobj :PyMethod_Type)
+    )
+        return nothing
+    end
+
+    ccall((@pysym :PyCallable_Check), Cint, (PyPtr,), m) == 1 && return m
+
+    return nothing
+end
+
 #########################################################################
 # Expose Python docstrings to the Julia doc system
 


Test patch:
diff --git a/test/runtests.jl b/test/runtests.jl
index 1c701d44..31249a99 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -170,6 +170,24 @@ const PyInt = pyversion < v"3" ? Int : Clonglong
 
     # fixme: is there any nontrivial showable test we can do?
     @test !showable("text/html", PyObject(1))
+    @testset "showable on type (#816)" begin
+        py"""
+        class Issue816(object):
+            def _repr_html_(self):
+                return "<h1>Issue816</h1>"
+
+        class CallableAsSpecialRepr(object):
+            _repr_html_ = Issue816()._repr_html_
+        """
+        @test showable("text/html", py"Issue816()")
+        @test !showable("text/html", py"Issue816")
+        @test showable("text/html", py"CallableAsSpecialRepr()")
+        if PyCall.pyversion_build < v"3"
+            @test_broken showable("text/html", py"CallableAsSpecialRepr")
+        else
+            @test showable("text/html", py"CallableAsSpecialRepr")
+        end
+    end
 
     # in Python 3, we need a specific encoding to write strings or bufferize them
     # (http://stackoverflow.com/questions/5471158/typeerror-str-does-not-support-the-buffer-interface)