remove getpy, now overload Py directly

Author: Christopher Doris

docs/src/conversion-to-python.md

@@ -36,11 +36,12 @@ See [here](@ref julia-wrappers) for an explanation of the `juliacall.*Value` wra
 
 ## [Custom rules](@id jl2py-conversion-custom)
 
-You may define a new conversion rule for your new type `T` by overloading `getpy(::T)` and
-possibly also `ispy(::T)` if it wraps a Python object.
+You may define a new conversion rule for your new type `T` by overloading `Py(::T)`.
+
+If `T` is a wrapper type (such as `PyList`) where `Py(x)` simply returns the stored Python
+object, then also define `ispy(::T) = true`.
 
 ```@docs
-PythonCall.getpy
 PythonCall.ispy
 ```
 

docs/src/pythoncall-reference.md

@@ -208,7 +208,7 @@ end
 ispy(x::MyType) = true
 
 # Says how to access the underlying Python object.
-getpy(x::MyType) = x.py
+Py(x::MyType) = x.py
 ```
 
 ## `@py` and `@pyconst`

docs/src/releasenotes.md

@@ -1,6 +1,9 @@
 # Release Notes
 
 ## Unreleased
+* **Breaking.** Removes `getpy`: you may now overload `Py` directly, which now need not
+  always return a new object (e.g. for singletons or wrappers).
+* **Breaking.** Conversion rules no longer take a new object every time.
 * **Breaking.** Improved Tables-interface support for `PyPandasDataFrame`: better inferred
   column types; better handling of non-string column names; columns are usually wrappers
   (`PyArray` or `PyList`). Constructor arguments have changed. Dict methods have been

src/Py.jl

@@ -23,7 +23,7 @@ pyisnull(x) = getptr(x) == C.PyNULL
 
 Get the underlying pointer from the Python object `x`.
 """
-getptr(x) = getptr(getpy(x)::Py)
+getptr(x) = ispy(x) ? getptr(Py(x)::Py) : throw(MethodError(getptr, (x,)))
 
 """
     Py(x)
@@ -36,10 +36,6 @@ Such an object supports attribute access (`obj.attr`), indexing (`obj[idx]`), ca
 (`obj(arg1, arg2)`), iteration (`for x in obj`), arithmetic (`obj + obj2`) and comparison
 (`obj > obj2`), among other things. These operations convert all their arguments to `Py` and
 return `Py`.
-
-!!! warning
-
-    Do not overload this function. To define a new conversion, overload [`getpy`](@ref).
 """
 mutable struct Py
     ptr :: C.PyPtr
@@ -56,7 +52,6 @@ function py_finalizer(x::Py)
 end
 
 ispy(::Py) = true
-getpy(x::Py) = x
 getptr(x::Py) = getfield(x, :ptr)
 
 setptr!(x::Py, ptr::C.PyPtr) = (setfield!(x, :ptr, ptr); x)
@@ -85,6 +80,8 @@ const PyNULL = pynew()
 
 pynew(ptr::C.PyPtr) = setptr!(pynew(), ptr)
 
+pynew(x::Py) = pynew(incref(getptr(x)))
+
 """
     pycopy!(dst::Py, src)
 
@@ -97,7 +94,7 @@ the top level then `pycopy!(x, pything())` inside `__init__()`.
 
 Assumes `dst` is NULL, otherwise a memory leak will occur.
 """
-pycopy!(dst, src) = GC.@preserve src setptr!(dst, incref(getptr(src)))
+pycopy!(dst::Py, src) = GC.@preserve src setptr!(dst, incref(getptr(src)))
 
 """
     pydel!(x::Py)
@@ -120,76 +117,44 @@ function pydel!(x::Py)
     ptr = getptr(x)
     if ptr != C.PyNULL
         C.Py_DecRef(ptr)
+        setptr!(x, C.PyNULL)
     end
-    pystolen!(x)
-end
-
-function pystolen!(x::Py)
-    setptr!(x, C.PyNULL)
     push!(PYNULL_CACHE, x)
-    nothing
+    return
 end
 
 macro autopy(args...)
     vs = args[1:end-1]
     ts = [Symbol(v, "_") for v in vs]
     body = args[end]
-    ans = gensym("ans")
+    # ans = gensym("ans")
     esc(quote
-        $([:($t = $ispy($v) ? $v : $Py($v)) for (t, v) in zip(ts, vs)]...)
-        $ans = $body
-        $([:($ispy($v) || $pydel!($t)) for (t, v) in zip(ts, vs)]...)
-        $ans
+        # $([:($t = $ispy($v) ? $v : $Py($v)) for (t, v) in zip(ts, vs)]...)
+        # $ans = $body
+        # $([:($ispy($v) || $pydel!($t)) for (t, v) in zip(ts, vs)]...)
+        # $ans
+        $([:($t = $Py($v)) for (t, v) in zip(ts, vs)]...)
+        $body
     end)
 end
 
-struct NewPy
-    py::Py
-    NewPy(py::Py) = new(py)
-end
-
-Py(x::Py) = GC.@preserve x pynew(incref(getptr(x))) # copy, because Py must always return a new object
-Py(x::NewPy) = x.py
-Py(x) = Py(getpy(x)::Union{Py,NewPy})
-
-"""
-    getpy(x)
-
-Convert `x` to a `Py`.
-
-Overload this function (not [`Py`](@ref)) to define a new conversion to Python.
-
-If `x` is a simple wrapper around a Python object (such as [`PyList`](@ref) or
-[`PyDict`](@ref)) then `getpy(x)` should return the Python object. You should also define
-`ispy(x) = true`. This means that when `x` is passed back to Python, the underlying object
-is used directly.
-
-### Optional optimization (for experts)
-
-If [`ispy(x)`](@ref) is false and the returned Julia object `ans` is not referenced anywhere
-else, in the sense that [`pydel!(ans)`](@ref) would be safe, you may instead return
-`NewPy(ans)`.
-
-This can avoid the Julia garbage collector in performance-critical code.
-
-If [`ispy(x)`](@ref) is true, you **must** return a [`Py`](@ref).
-"""
-getpy(x) = ispy(x) ? throw(MethodError(getpy, (x,))) : NewPy(pyjl(x))
-getpy(x::Nothing) = pybuiltins.None
-getpy(x::Bool) = NewPy(pybool(x))
-getpy(x::Union{String, SubString{String}, Char}) = NewPy(pystr(x))
-getpy(x::Base.CodeUnits{UInt8, String}) = NewPy(pybytes(x))
-getpy(x::Base.CodeUnits{UInt8, SubString{String}}) = NewPy(pybytes(x))
-getpy(x::Tuple) = NewPy(pytuple_fromiter(x))
-getpy(x::Pair) = NewPy(pytuple_fromiter(x))
-getpy(x::Union{Int8,Int16,Int32,Int64,Int128,UInt8,UInt16,UInt32,UInt64,UInt128,BigInt}) = NewPy(pyint(x))
-getpy(x::Rational{<:Union{Int8,Int16,Int32,Int64,Int128,UInt8,UInt16,UInt32,UInt64,UInt128,BigInt}}) = NewPy(pyfraction(x))
-getpy(x::Union{Float16,Float32,Float64}) = NewPy(pyfloat(x))
-getpy(x::Complex{<:Union{Float16,Float32,Float64}}) = NewPy(pycomplex(x))
-getpy(x::AbstractRange{<:Union{Int8,Int16,Int32,Int64,Int128,UInt8,UInt16,UInt32,UInt64,UInt128,BigInt}}) = NewPy(pyrange_fromrange(x))
-getpy(x::Date) = NewPy(pydate(x))
-getpy(x::Time) = NewPy(pytime(x))
-getpy(x::DateTime) = NewPy(pydatetime(x))
+Py(x::Py) = x
+Py(x::Nothing) = pybuiltins.None
+Py(x::Bool) = x ? pybuiltins.True : pybuiltins.False
+Py(x::Union{String, SubString{String}, Char}) = pystr(x)
+Py(x::Base.CodeUnits{UInt8, String}) = pybytes(x)
+Py(x::Base.CodeUnits{UInt8, SubString{String}}) = pybytes(x)
+Py(x::Tuple) = pytuple_fromiter(x)
+Py(x::Pair) = pytuple_fromiter(x)
+Py(x::Union{Int8,Int16,Int32,Int64,Int128,UInt8,UInt16,UInt32,UInt64,UInt128,BigInt}) = pyint(x)
+Py(x::Rational{<:Union{Int8,Int16,Int32,Int64,Int128,UInt8,UInt16,UInt32,UInt64,UInt128,BigInt}}) = pyfraction(x)
+Py(x::Union{Float16,Float32,Float64}) = pyfloat(x)
+Py(x::Complex{<:Union{Float16,Float32,Float64}}) = pycomplex(x)
+Py(x::AbstractRange{<:Union{Int8,Int16,Int32,Int64,Int128,UInt8,UInt16,UInt32,UInt64,UInt128,BigInt}}) = pyrange_fromrange(x)
+Py(x::Date) = pydate(x)
+Py(x::Time) = pytime(x)
+Py(x::DateTime) = pydatetime(x)
+Py(x) = ispy(x) ? throw(MethodError(Py, (x,))) : pyjl(x)
 
 Base.string(x::Py) = pyisnull(x) ? "<py NULL>" : pystr(String, x)
 Base.print(io::IO, x::Py) = print(io, string(x))

src/abstract/collection.jl

@@ -119,7 +119,6 @@ for N in 0:16
             :($z = @pyconvert_and_del($T, pytuple_getitem(xs, $(i-1))))
             for (i, T, z) in zip(1:N, Ts, zs)
         )...)
-        pydel!(xs)
         return pyconvert_return(($(zs...),))
     end
     # Tuple with N elements plus Vararg
@@ -136,7 +135,6 @@ for N in 0:16
             v = @pyconvert_and_del(V, pytuple_getitem(xs, i-1))
             push!(vs, v)
         end
-        pydel!(xs)
         return pyconvert_return(($(zs...), vs...))
     end
 end

src/abstract/object.jl

@@ -240,34 +240,39 @@ export pycall
 Equivalent to `x == y` in Python. The second form converts to `Bool`.
 """
 pyeq(x, y) = pynew(errcheck(@autopy x y C.PyObject_RichCompare(getptr(x_), getptr(y_), C.Py_EQ)))
+
 """
     pyne(x, y)
     pyne(Bool, x, y)
 
 Equivalent to `x != y` in Python. The second form converts to `Bool`.
 """
 pyne(x, y) = pynew(errcheck(@autopy x y C.PyObject_RichCompare(getptr(x_), getptr(y_), C.Py_NE)))
+
 """
     pyle(x, y)
     pyle(Bool, x, y)
 
 Equivalent to `x <= y` in Python. The second form converts to `Bool`.
 """
 pyle(x, y) = pynew(errcheck(@autopy x y C.PyObject_RichCompare(getptr(x_), getptr(y_), C.Py_LE)))
+
 """
     pylt(x, y)
     pylt(Bool, x, y)
 
 Equivalent to `x < y` in Python. The second form converts to `Bool`.
 """
 pylt(x, y) = pynew(errcheck(@autopy x y C.PyObject_RichCompare(getptr(x_), getptr(y_), C.Py_LT)))
+
 """
     pyge(x, y)
     pyge(Bool, x, y)
 
 Equivalent to `x >= y` in Python. The second form converts to `Bool`.
 """
 pyge(x, y) = pynew(errcheck(@autopy x y C.PyObject_RichCompare(getptr(x_), getptr(y_), C.Py_GE)))
+
 """
     pygt(x, y)
     pygt(Bool, x, y)
@@ -283,7 +288,7 @@ pyge(::Type{Bool}, x, y) = errcheck(@autopy x y C.PyObject_RichCompareBool(getpt
 pygt(::Type{Bool}, x, y) = errcheck(@autopy x y C.PyObject_RichCompareBool(getptr(x_), getptr(y_), C.Py_GT)) == 1
 export pyeq, pyne, pyle, pylt, pyge, pygt
 
-pyconvert_rule_object(::Type{Py}, x) = pyconvert_return(Py(x))
+pyconvert_rule_object(::Type{Py}, x::Py) = pyconvert_return(x)
 
 """
     pycontains(x, v)

src/concrete/bool.jl

@@ -3,7 +3,7 @@
 
 Convert `x` to a Python `bool`.
 """
-pybool(x::Bool=false) = Py(x ? pybuiltins.True : pybuiltins.False)
+pybool(x::Bool=false) = pynew(x ? pybuiltins.True : pybuiltins.False)
 pybool(x::Number) = pybool(!iszero(x))
 pybool(x) = pybuiltins.bool(x)
 export pybool
@@ -23,7 +23,6 @@ pybool_asbool(x) =
 
 function pyconvert_rule_bool(::Type{T}, x::Py) where {T<:Number}
     val = pybool_asbool(x)
-    pydel!(x)
     if T in (Bool, Int8, Int16, Int32, Int64, Int128, UInt8, UInt16, UInt32, UInt64, UInt128, BigInt)
         pyconvert_return(T(val))
     else

src/concrete/code.jl

@@ -9,9 +9,9 @@ function _pyeval_args(globals, locals)
         ArgumentError("globals must be a module or a Python dict")
     end
     if locals === nothing
-        locals_ = Py(globals_)
+        locals_ = pynew(Py(globals_))
     elseif ispy(locals)
-        locals_ = Py(locals)
+        locals_ = pynew(Py(locals))
     else
         locals_ = pydict(locals)
     end

src/concrete/float.jl

@@ -16,7 +16,6 @@ pyfloat_asdouble(x) = errcheck_ambig(@autopy x C.PyFloat_AsDouble(getptr(x_)))
 
 function pyconvert_rule_float(::Type{T}, x::Py) where {T<:Number}
     val = pyfloat_asdouble(x)
-    pydel!(x)
     if T in (Float16, Float32, Float64, BigFloat)
         pyconvert_return(T(val))
     else
@@ -28,7 +27,6 @@ end
 # so we allow converting it to Nothing or Missing
 function pyconvert_rule_float(::Type{Nothing}, x::Py)
     val = pyfloat_asdouble(x)
-    pydel!(x)
     if isnan(val)
         pyconvert_return(nothing)
     else
@@ -38,7 +36,6 @@ end
 
 function pyconvert_rule_float(::Type{Missing}, x::Py)
     val = pyfloat_asdouble(x)
-    pydel!(x)
     if isnan(val)
         pyconvert_return(missing)
     else

src/concrete/int.jl

@@ -39,7 +39,6 @@ pyconvert_rule_int(::Type{T}, x::Py) where {T<:Number} = begin
     v = T <: Unsigned ? C.PyLong_AsUnsignedLongLong(getptr(x)) : C.PyLong_AsLongLong(getptr(x))
     if !iserrset_ambig(v)
         # success
-        pydel!(x)
         return pyconvert_tryconvert(T, v)
     elseif errmatches(pybuiltins.OverflowError)
         # overflows Clonglong or Culonglong
@@ -60,20 +59,17 @@ pyconvert_rule_int(::Type{T}, x::Py) where {T<:Number} = begin
            typemin(typeof(v)) ≤ typemin(T) &&
            typemax(T) ≤ typemax(typeof(v))
             # definitely overflows S, give up now
-            pydel!(x)
             return pyconvert_unconverted()
         else
             # try converting -> int -> str -> BigInt -> T
             x_int = pyint(x)
-            pydel!(x)
             x_str = pystr(String, x_int)
             pydel!(x_int)
             v = parse(BigInt, x_str)
             return pyconvert_tryconvert(T, v)
         end
     else
         # other error
-        pydel!(x)
         pythrow()
     end
 end