Repository: JuliaPy/PyCall.jl
Pull request #997 (2022-07-30 08:21:48)

fix getproperty exceptions
this throws KeyError as before when the attribute isn't there (AttributeError in python)
when the attribute is present, but its computation throws a python exception, this PR rethrows it in julia

fixes https://github.com/JuliaPy/PyCall.jl/issues/696

codecov-commenter: # [Codecov](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#997](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (e0f2cb0) into [master](https://codecov.io/gh/JuliaPy/PyCall.jl/commit/b27bd5a9ecabec4a30ba6062c3754768746e3adb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (b27bd5a) will **decrease** coverage by `3.05%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #997      +/-   ##
==========================================
- Coverage   68.36%   65.31%   -3.06%     
==========================================
  Files          20       20              
  Lines        2023     1920     -103     
==========================================
- Hits         1383     1254     -129     
- Misses        640      666      +26     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `65.31% <100.00%> (-3.06%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [src/PyCall.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL1B5Q2FsbC5qbA==) | `61.85% <100.00%> (-8.84%)` | :arrow_down: |
| [src/startup.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3N0YXJ0dXAuamw=) | `0.00% <0.00%> (-53.13%)` | :arrow_down: |
| [src/pyinit.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3B5aW5pdC5qbA==) | `66.00% <0.00%> (-17.00%)` | :arrow_down: |
| [src/exception.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2V4Y2VwdGlvbi5qbA==) | `56.25% <0.00%> (-4.50%)` | :arrow_down: |
| [src/conversions.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbnZlcnNpb25zLmps) | `60.15% <0.00%> (-3.42%)` | :arrow_down: |
| [src/numpy.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL251bXB5Lmps) | `72.36% <0.00%> (-2.95%)` | :arrow_down: |
| [src/pyeval.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3B5ZXZhbC5qbA==) | `70.37% <0.00%> (-1.71%)` | :arrow_down: |
| [src/pytype.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3B5dHlwZS5qbA==) | `80.45% <0.00%> (-1.36%)` | :arrow_down: |
| [src/io.jl](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2lvLmps) | `82.60% <0.00%> (-0.73%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [b27bd5a...e0f2cb0](https://codecov.io/gh/JuliaPy/PyCall.jl/pull/997?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).

stevengj: Looks good, modulo a couple of minor tweaks noted above.

Patch:
diff --git a/src/PyCall.jl b/src/PyCall.jl
index c09cc20..7f0317e 100644
--- a/src/PyCall.jl
+++ b/src/PyCall.jl
@@ -298,7 +298,13 @@ end
 function _getproperty(o::PyObject, s::Union{AbstractString,Symbol})
     ispynull(o) && throw(ArgumentError("ref of NULL PyObject"))
     p = ccall((@pysym :PyObject_GetAttrString), PyPtr, (PyPtr, Cstring), o, s)
-    p == C_NULL && pyerr_clear()
+    if p == C_NULL && pyerr_occurred()
+        e = pyerror("PyObject_GetAttrString")
+        if PyPtr(e.T) != @pyglobalobjptr(:PyExc_AttributeError)
+            throw(e)
+        end
+        pyerr_clear()
+    end
     return p
 end
 
@@ -327,15 +333,17 @@ setproperty!(o::PyObject, s::Symbol, v) = _setproperty!(o,s,v)
 setproperty!(o::PyObject, s::AbstractString, v) = _setproperty!(o,s,v)
 
 function _setproperty!(o::PyObject, s::Union{Symbol,AbstractString}, v)
-    if ispynull(o)
-        throw(ArgumentError("assign of NULL PyObject"))
-    end
-    if -1 == ccall((@pysym :PyObject_SetAttrString), Cint,
-                   (PyPtr, Cstring, PyPtr), o, s, PyObject(v))
+    ispynull(o) && throw(ArgumentError("assign of NULL PyObject"))
+    p = ccall((@pysym :PyObject_SetAttrString), Cint, (PyPtr, Cstring, PyPtr), o, s, PyObject(v))
+    if p == -1 && pyerr_occurred()
+        e = pyerror("PyObject_SetAttrString")
+        if PyPtr(e.T) != @pyglobalobjptr(:PyExc_AttributeError)
+            throw(e)
+        end
         pyerr_clear()
         throw(KeyError(s))
     end
-    o
+    return o
 end
 
 function getindex(o::PyObject, s::T) where T<:Union{Symbol, AbstractString}
diff --git a/src/gui.jl b/src/gui.jl
index ff76b12..9ed0431 100644
--- a/src/gui.jl
+++ b/src/gui.jl
@@ -1,6 +1,6 @@
 # GUI event loops and toolkit integration for Python, most importantly
 # to support plotting in a window without blocking.  Currently, we
-# support wxWidgets, Qt5, Qt4, and GTK+.
+# support wxWidgets, Qt4, Qt5, Qt6, and GTK+.
 
 ############################################################################
 
@@ -26,11 +26,14 @@ pygui_works(gui::Symbol) = gui == :default ||
      (gui == :tk && pyexists(tkinter_name())) ||
      (gui == :qt_pyqt4 && pyexists("PyQt4")) ||
      (gui == :qt_pyqt5 && pyexists("PyQt5")) ||
+     (gui == :qt_pyqt6 && pyexists("PyQt6")) ||
      (gui == :qt_pyside && pyexists("PySide")) ||
      (gui == :qt_pyside2 && pyexists("PySide2")) ||
+     (gui == :qt_pyside6 && pyexists("PySide6")) ||
      (gui == :qt4 && (pyexists("PyQt4") || pyexists("PySide"))) ||
      (gui == :qt5 && (pyexists("PyQt5") || pyexists("PySide2"))) ||
-     (gui == :qt && (pyexists("PyQt5") || pyexists("PyQt4") || pyexists("PySide") || pyexists("PySide2"))))
+     (gui == :qt6 && (pyexists("PyQt6") || pyexists("PySide6"))) ||
+     (gui == :qt && (pyexists("PyQt6") || pyexists("PyQt5") || pyexists("PyQt4") || pyexists("PySide") || pyexists("PySide2") || pyexists("PySide6"))))
 
 # get or set the default GUI; doesn't affect running GUI
 """
@@ -55,7 +58,7 @@ end
 function pygui(g::Symbol)
     global gui
     if g != gui::Symbol
-        if !(g in (:wx,:gtk,:gtk3,:tk,:qt,:qt4,:qt5,:qt_pyqt5,:qt_pyqt4,:qt_pyside,:qt_pyside2,:default))
+        if !(g in (:wx,:gtk,:gtk3,:tk,:qt,:qt4,:qt5,:qt6,:qt_pyqt4,:qt_pyqt5,:qt_pyqt6,:qt_pyside,:qt_pyside2,:qt_pyside6,:default))
             throw(ArgumentError("invalid gui $g"))
         elseif !pygui_works(g)
             error("Python GUI toolkit for $g is not installed.")
@@ -136,7 +139,7 @@ function fixqtpath(qtconf=joinpath(dirname(pyprogramname),"qt.conf"))
     return false
 end
 
-# Qt: (PyQt5, PyQt4, or PySide module)
+# Qt: (PyQt6, PyQt5, PyQt4, or PySide module)
 function qt_eventloop(QtCore::PyObject, sec::Real=50e-3)
     fixqtpath()
     instance = QtCore."QCoreApplication"."instance"
@@ -157,7 +160,7 @@ qt_eventloop(QtModule::AbstractString, sec::Real=50e-3) =
     qt_eventloop(pyimport("$QtModule.QtCore"), sec)
 
 function qt_eventloop(sec::Real=50e-3)
-    for QtModule in ("PyQt5", "PyQt4", "PySide", "PySide2")
+    for QtModule in ("PyQt6", "PyQt5", "PyQt4", "PySide", "PySide2", "PySide6")
         try
             return qt_eventloop(QtModule, sec)
         catch
@@ -232,11 +235,15 @@ function pygui_start(gui::Symbol=pygui(), sec::Real=50e-3)
         elseif gui == :qt_pyqt4
             eventloops[gui] = qt_eventloop("PyQt4", sec)
         elseif gui == :qt_pyqt5
-            eventloops[gui] = qt_eventloop("PyQt5", sec) 
+            eventloops[gui] = qt_eventloop("PyQt5", sec)
+        elseif gui == :qt_pyqt6
+            eventloops[gui] = qt_eventloop("PyQt6", sec)
         elseif gui == :qt_pyside
             eventloops[gui] = qt_eventloop("PySide", sec)
         elseif gui == :qt_pyside2
             eventloops[gui] = qt_eventloop("PySide2", sec)
+        elseif gui == :qt_pyside6
+            eventloops[gui] = qt_eventloop("PySide6", sec)
         elseif gui == :qt4
             try
                 eventloops[gui] = qt_eventloop("PyQt4", sec)
@@ -249,6 +256,12 @@ function pygui_start(gui::Symbol=pygui(), sec::Real=50e-3)
             catch
                 eventloops[gui] = qt_eventloop("PySide2", sec)
             end
+        elseif gui == :qt6
+            try
+                eventloops[gui] = qt_eventloop("PyQt6", sec)
+            catch
+                eventloops[gui] = qt_eventloop("PySide6", sec)
+            end
         elseif gui == :qt
             eventloops[gui] = qt_eventloop(sec)
         else


Test patch:
diff --git a/test/runtests.jl b/test/runtests.jl
index 4594cc4..e16b31a 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -829,3 +829,26 @@ end
         @test py"bar2" == 2
     end
 end
+
+@testset "proper exception raised" begin
+    py"""
+    class A:
+        def __getattr__(self, name):
+            if name == "a":
+                raise ValueError(name)
+            else:
+                raise AttributeError()
+
+        def __setattr__(self, name, value):
+            if value == 0:
+                raise ValueError(value)
+            else:
+                raise AttributeError()
+    """
+    a = py"A"()
+    @test_throws PyCall.PyError a.a
+    @test_throws KeyError a.b
+
+    @test_throws PyCall.PyError a.a = 0
+    @test_throws KeyError a.a = 1
+end