Conversation
34eff30 to
ac40869
Compare
|
@filmor If I understand this correctly, one wouldn't even need the dynamic.py script anymore for DynamicObject access to work, correct? That seems great :) |
|
@spkl Yes, the dynamic support would be integrated. To build a wheel, the simplest way is to get |
|
OK so I did a little digging and found out that I can Getting the value of dynamic properties seems to work, but I encountered a problem when setting dynamic properties. This is what I observed when running a script like this: x = obj.MyProp
obj.MyProp = None
y = obj.MyPropFor line 1, |
fdfa219 to
6d1572e
Compare
|
@spkl Could you test again whether this works for you? |
|
Will do ASAP and let you know |
|
OK @filmor, this is pretty great. With the current version from this branch and with all previous workarounds removed, all of our test cases pass. 🎉 Performance is also improved, which I could see in our two dedicated performance tests. These tests don't strictly measure pythonnet performance, but that is a significant part of them. Numbers are normalized to fractions, lower is better.
There is one significant behavioral difference that I observed: With past solutions, there was always an AttributeError when setting an attribute for which TrySetMember returns false (= which does not exist). With this branch, the attribute is now set on the Python side without any error. This can be problematic, because it can mask errors / typos in scripts and make troubleshooting harder. I don't know if this was intended. If it is, we can probably find a different solution. I tried to find a workaround for that behavior and discovered something else where I'm not sure if it's correct: When throwing an exception in TrySetMember, the process crashes with a .NET exception, so there is no Python traceback to pinpoint the error location. In contrast, when throwing an exception in TryGetMember, it comes up as an AttributeError with Python traceback output. Because we support running in different environments and Python versions, we probably need to implement automatic switching between DynamicObject workaround and first-class DLR support in pythonnet. Is there a way to find out if the currently loaded pythonnet module has DLR support? An information about the version would also work, but pythonnet currently has no |
|
I'll try to handle these cases, definitely not intentional :) If you need to get a package's version in your code, please use |
The dynamic getter swallowed any exception from TryGetMember and returned default to Python with the prior AttributeError still set, so user code observed a misleading AttributeError instead of the real failure. Set a Python exception in the catch arm. We use RuntimeError with the message string rather than Converter.ToPython(e) because wrapping the CLR exception object can trigger type initialisation that re-enters this same slot on the live dynamic object, producing infinite recursion. Mirrors the symmetry already present in the setter (pythonnet#2706 review, @lostmsu) and adds a regression test alongside the existing ThrowingSetDynamicObject coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The dynamic getter swallowed any exception from TryGetMember and returned default to Python with the prior AttributeError still set, so user code observed a misleading AttributeError instead of the real failure. Set a Python exception in the catch arm. We use RuntimeError with the message string rather than Converter.ToPython(e) because wrapping the CLR exception object can trigger type initialisation that re-enters this same slot on the live dynamic object, producing infinite recursion. Mirrors the symmetry already present in the setter (pythonnet#2706 review, @lostmsu) and adds a regression test alongside the existing ThrowingSetDynamicObject coverage.
|
#2718 should fix the |
The dynamic getter swallowed any exception from TryGetMember and returned default to Python with the prior AttributeError still set, so user code observed a misleading AttributeError instead of the real failure. Set a Python exception in the catch arm. We use RuntimeError with the message string rather than Converter.ToPython(e) because wrapping the CLR exception object can trigger type initialisation that re-enters this same slot on the live dynamic object, producing infinite recursion. Mirrors the symmetry already present in the setter (#2706 review, @lostmsu) and adds a regression test alongside the existing ThrowingSetDynamicObject coverage.
|
You want to rebase this and retest with the |
- Cache HasClrMember reflection per (Type, name) so tp_getattro_dlr_proxy / tp_setattro_dlr_proxy avoid repeated GetMember() calls on every attribute access of DLR-aware objects. - Mirror tp_setattro_dlr_proxy's catch arm to the getter's safer SetError(RuntimeError, e.Message) shape instead of SetError(Exception), keeping both slots re-entry-safe on live dynamic objects. Related to pythonnet#2706.
The dynamic getter swallowed any exception from TryGetMember and returned default to Python with the prior AttributeError still set, so user code observed a misleading AttributeError instead of the real failure. Set a Python exception in the catch arm. We use RuntimeError with the message string rather than Converter.ToPython(e) because wrapping the CLR exception object can trigger type initialisation that re-enters this same slot on the live dynamic object, producing infinite recursion. Mirrors the symmetry already present in the setter (#2706 review, @lostmsu) and adds a regression test alongside the existing ThrowingSetDynamicObject coverage.
- Cache HasClrMember reflection per (Type, name) so tp_getattro_dlr_proxy / tp_setattro_dlr_proxy avoid repeated GetMember() calls on every attribute access of DLR-aware objects. - Mirror tp_setattro_dlr_proxy's catch arm to the getter's safer SetError(RuntimeError, e.Message) shape instead of SetError(Exception), keeping both slots re-entry-safe on live dynamic objects. Related to #2706.
The dynamic getter swallowed any exception from TryGetMember and returned default to Python with the prior AttributeError still set, so user code observed a misleading AttributeError instead of the real failure. Set a Python exception in the catch arm. We use RuntimeError with the message string rather than Converter.ToPython(e) because wrapping the CLR exception object can trigger type initialisation that re-enters this same slot on the live dynamic object, producing infinite recursion. Mirrors the symmetry already present in the setter (#2706 review, @lostmsu) and adds a regression test alongside the existing ThrowingSetDynamicObject coverage.
- Cache HasClrMember reflection per (Type, name) so tp_getattro_dlr_proxy / tp_setattro_dlr_proxy avoid repeated GetMember() calls on every attribute access of DLR-aware objects. - Mirror tp_setattro_dlr_proxy's catch arm to the getter's safer SetError(RuntimeError, e.Message) shape instead of SetError(Exception), keeping both slots re-entry-safe on live dynamic objects. Related to #2706.
The dynamic getter swallowed any exception from TryGetMember and returned default to Python with the prior AttributeError still set, so user code observed a misleading AttributeError instead of the real failure. Set a Python exception in the catch arm. We use RuntimeError with the message string rather than Converter.ToPython(e) because wrapping the CLR exception object can trigger type initialisation that re-enters this same slot on the live dynamic object, producing infinite recursion. Mirrors the symmetry already present in the setter (#2706 review, @lostmsu) and adds a regression test alongside the existing ThrowingSetDynamicObject coverage.
- Cache HasClrMember reflection per (Type, name) so tp_getattro_dlr_proxy / tp_setattro_dlr_proxy avoid repeated GetMember() calls on every attribute access of DLR-aware objects. - Mirror tp_setattro_dlr_proxy's catch arm to the getter's safer SetError(RuntimeError, e.Message) shape instead of SetError(Exception), keeping both slots re-entry-safe on live dynamic objects. Related to #2706.
|
@spkl Can you please test out the latest version? |
|
@filmor All good from my side 👍 |
- Catch exceptions in TrySet/DeleteMember - Convert the exceptions into Python exceptions - Add tests for the remaining cases - Add a note on why the field has to be lazily initialized (general issue with derived classes)
The dynamic getter swallowed any exception from TryGetMember and returned default to Python with the prior AttributeError still set, so user code observed a misleading AttributeError instead of the real failure. Set a Python exception in the catch arm. We use RuntimeError with the message string rather than Converter.ToPython(e) because wrapping the CLR exception object can trigger type initialisation that re-enters this same slot on the live dynamic object, producing infinite recursion. Mirrors the symmetry already present in the setter (#2706 review, @lostmsu) and adds a regression test alongside the existing ThrowingSetDynamicObject coverage.
- Cache HasClrMember reflection per (Type, name) so tp_getattro_dlr_proxy / tp_setattro_dlr_proxy avoid repeated GetMember() calls on every attribute access of DLR-aware objects. - Mirror tp_setattro_dlr_proxy's catch arm to the getter's safer SetError(RuntimeError, e.Message) shape instead of SetError(Exception), keeping both slots re-entry-safe on live dynamic objects. Related to #2706.
Implements #72 and should thus fix #2696.
@spkl Could you try out this branch?