Skip to content

Commit 6ac987f

Browse files
committed
Address peer review
1 parent 210e50e commit 6ac987f

File tree

3 files changed

+20
-29
lines changed

3 files changed

+20
-29
lines changed

src/embed_tests/TestPropertyAccess.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1091,7 +1091,7 @@ def GetValue(self, fixture):
10911091
var result = model.GetValue(fixture) as PyObject;
10921092
Assert.IsFalse(result.IsNone());
10931093
Assert.AreEqual(result.PyType, Exceptions.AttributeError);
1094-
Assert.AreEqual("'Python.EmbeddingTest.TestPropertyAccess+DynamicFixture' object has no attribute 'AnotherProperty'",
1094+
Assert.AreEqual("'DynamicFixture' object has no attribute 'AnotherProperty'",
10951095
result.ToString());
10961096
}
10971097
}

src/runtime/ClassManager.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,6 @@ internal static ClassBase CreateClass(Type type)
184184
impl = new KeyValuePairEnumerableObject(type);
185185
}
186186

187-
else if (typeof(IDynamicMetaObjectProvider).IsAssignableFrom(type))
188-
{
189-
impl = new DynamicClassObject(type);
190-
}
191-
192187
else if (type.IsInterface)
193188
{
194189
impl = new InterfaceObject(type);
@@ -207,6 +202,11 @@ internal static ClassBase CreateClass(Type type)
207202
impl = new ClassDerivedObject(type);
208203
}
209204

205+
else if (typeof(IDynamicMetaObjectProvider).IsAssignableFrom(type))
206+
{
207+
impl = new DynamicClassObject(type);
208+
}
209+
210210
else
211211
{
212212
impl = new ClassObject(type);

src/runtime/Types/DynamicClassObject.cs

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -67,33 +67,24 @@ public static NewReference tp_getattro(BorrowedReference ob, BorrowedReference k
6767
{
6868
var result = Runtime.PyObject_GenericGetAttr(ob, key);
6969

70-
// Property not found, but it can still be a dynamic one if the object is an IDynamicMetaObjectProvider
71-
if (result.IsNull())
70+
// If AttributeError was raised, we try to get the attribute from the managed object dynamic properties.
71+
if (Exceptions.ExceptionMatches(Exceptions.AttributeError))
7272
{
7373
var clrObj = (CLRObject)GetManagedObject(ob)!;
74-
if (clrObj?.inst is IDynamicMetaObjectProvider)
75-
{
76-
77-
// The call to Runtime.PyObject_GenericGetAttr above ended up with an AttributeError
78-
// for dynamic properties since they are not found in the C# object definition.
79-
if (Exceptions.ExceptionMatches(Exceptions.AttributeError))
80-
{
81-
Exceptions.Clear();
82-
}
8374

84-
var name = Runtime.GetManagedString(key);
85-
var clrObjectType = clrObj.inst.GetType();
86-
var callSite = GetAttrCallSite(name, clrObjectType);
75+
var name = Runtime.GetManagedString(key);
76+
var clrObjectType = clrObj.inst.GetType();
77+
var callSite = GetAttrCallSite(name, clrObjectType);
8778

88-
try
89-
{
90-
var res = callSite.Target(callSite, clrObj.inst);
91-
return Converter.ToPython(res);
92-
}
93-
catch (RuntimeBinder.RuntimeBinderException)
94-
{
95-
Exceptions.SetError(Exceptions.AttributeError, $"'{clrObjectType}' object has no attribute '{name}'");
96-
}
79+
try
80+
{
81+
var res = callSite.Target(callSite, clrObj.inst);
82+
Exceptions.Clear();
83+
result = Converter.ToPython(res);
84+
}
85+
catch (RuntimeBinder.RuntimeBinderException)
86+
{
87+
// Do nothing, AttributeError was already raised in Python side and it was not cleared.
9788
}
9889
}
9990

0 commit comments

Comments
 (0)