Skip to content

Commit dc5fc78

Browse files
Christopher-ChianelliChristopher-Chianelli
authored
fix: Use Python object for Score, handle descriptor correctly, do not copy parent fields (#66)
- Using Python object for Score allows Python libraries to interpret the score, allowing the @planning_solution class, justification and other Python classes that has a score to be used by Pydantic and the like. This also allows `Score | None` to be used in type annotations. - Handle descriptors correctly. Previously, instance fields were generated for them (which is incorrect, since they are functions to be called by __getattribute__). Now, the static attributes of a Python class are checked, and all static attributes that are descriptors are removed from the instance field candidate set. This uses an API that was added in Python 3.11 (inspect.getmembers_static). We do a best attempt in Python 3.10 to resolve descriptor correctly. This causes test failures for Python 3.10 in jpyinterpreter but not for timefold.solver. Tests in Python 3.11 and above all pass. - Previously, parent fields were sometimes added to the instance field candidate set, causing some fields to incorrectly be None when unwrapping a PythonLikeObject. Now, all parent fields are removed from the instance field candidate set. - Made toString() call $method$__str__(), and change the return type of $method$__str__() to PythonString so overrides work correctly.
1 parent 97bf0f7 commit dc5fc78

File tree

52 files changed

+1439
-294
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1439
-294
lines changed

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/PythonClassTranslator.java

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static ai.timefold.jpyinterpreter.PythonBytecodeToJavaBytecodeTranslator.ARGUMENT_SPEC_INSTANCE_FIELD_NAME;
44

5+
import java.lang.reflect.Field;
56
import java.lang.reflect.InvocationTargetException;
67
import java.lang.reflect.Modifier;
78
import java.util.ArrayList;
@@ -38,6 +39,7 @@
3839
import ai.timefold.jpyinterpreter.types.BuiltinTypes;
3940
import ai.timefold.jpyinterpreter.types.CPythonBackedPythonLikeObject;
4041
import ai.timefold.jpyinterpreter.types.GeneratedFunctionMethodReference;
42+
import ai.timefold.jpyinterpreter.types.PythonJavaTypeMapping;
4143
import ai.timefold.jpyinterpreter.types.PythonLikeFunction;
4244
import ai.timefold.jpyinterpreter.types.PythonLikeType;
4345
import ai.timefold.jpyinterpreter.types.PythonNone;
@@ -65,6 +67,7 @@ public class PythonClassTranslator {
6567
public static final String TYPE_FIELD_NAME = "$TYPE";
6668
public static final String CPYTHON_TYPE_FIELD_NAME = "$CPYTHON_TYPE";
6769
private static final String JAVA_METHOD_PREFIX = "$method$";
70+
private static final String PYTHON_JAVA_TYPE_MAPPING_PREFIX = "$pythonJavaTypeMapping";
6871

6972
public record PreparedClassInfo(PythonLikeType type, String className, String classInternalName) {
7073
}
@@ -205,7 +208,25 @@ public static PythonLikeType translatePythonClass(PythonCompiledClass pythonComp
205208
}
206209
}
207210

211+
for (int i = 0; i < pythonCompiledClass.pythonJavaTypeMappings.size(); i++) {
212+
classWriter.visitField(Modifier.PUBLIC | Modifier.STATIC, PYTHON_JAVA_TYPE_MAPPING_PREFIX + i,
213+
Type.getDescriptor(PythonJavaTypeMapping.class), null, null);
214+
}
215+
208216
Map<String, PythonLikeType> attributeNameToTypeMap = new HashMap<>();
217+
instanceAttributeSet.removeAll(pythonCompiledClass.staticAttributeDescriptorNames);
218+
try {
219+
var parentClass = superClassType.getJavaClass();
220+
while (parentClass != Object.class) {
221+
for (Field field : parentClass.getFields()) {
222+
instanceAttributeSet.remove(field.getName());
223+
}
224+
parentClass = parentClass.getSuperclass();
225+
}
226+
} catch (ClassNotFoundException e) {
227+
throw new IllegalStateException(e);
228+
}
229+
209230
for (String attributeName : instanceAttributeSet) {
210231
var typeHint = pythonCompiledClass.typeAnnotations.getOrDefault(attributeName,
211232
TypeHint.withoutAnnotations(BuiltinTypes.BASE_TYPE));
@@ -243,8 +264,8 @@ public static PythonLikeType translatePythonClass(PythonCompiledClass pythonComp
243264
}
244265
fieldVisitor.visitEnd();
245266

246-
createJavaGetterSetter(classWriter, preparedClassInfo,
247-
attributeName,
267+
createJavaGetterSetter(classWriter, pythonCompiledClass,
268+
preparedClassInfo, attributeName,
248269
Type.getType(javaFieldTypeDescriptor),
249270
Type.getType(getterTypeDescriptor),
250271
signature,
@@ -368,6 +389,10 @@ public static PythonLikeType translatePythonClass(PythonCompiledClass pythonComp
368389
generatedClass = (Class<? extends PythonLikeObject>) BuiltinTypes.asmClassLoader.loadClass(className);
369390
generatedClass.getField(TYPE_FIELD_NAME).set(null, pythonLikeType);
370391
generatedClass.getField(CPYTHON_TYPE_FIELD_NAME).set(null, pythonCompiledClass.binaryType);
392+
for (int i = 0; i < pythonCompiledClass.pythonJavaTypeMappings.size(); i++) {
393+
generatedClass.getField(PYTHON_JAVA_TYPE_MAPPING_PREFIX + i)
394+
.set(null, pythonCompiledClass.pythonJavaTypeMappings.get(i));
395+
}
371396
} catch (ClassNotFoundException e) {
372397
throw new IllegalStateException("Impossible State: Unable to load generated class (" +
373398
className + ") despite it being just generated.", e);
@@ -785,16 +810,31 @@ private static PythonLikeFunction createConstructor(String classInternalName,
785810
}
786811
}
787812

813+
private record MatchedMapping(int index, PythonJavaTypeMapping<?, ?> pythonJavaTypeMapping) {
814+
}
815+
788816
private static void createJavaGetterSetter(ClassWriter classWriter,
817+
PythonCompiledClass pythonCompiledClass,
789818
PreparedClassInfo preparedClassInfo,
790819
String attributeName, Type attributeType, Type getterType,
791820
String signature,
792821
TypeHint typeHint) {
793-
createJavaGetter(classWriter, preparedClassInfo, attributeName, attributeType, getterType, signature, typeHint);
794-
createJavaSetter(classWriter, preparedClassInfo, attributeName, attributeType, getterType, signature, typeHint);
822+
MatchedMapping matchedMapping = null;
823+
for (int i = 0; i < pythonCompiledClass.pythonJavaTypeMappings.size(); i++) {
824+
var mapping = pythonCompiledClass.pythonJavaTypeMappings.get(i);
825+
if (mapping.getPythonType().equals(typeHint.javaGetterType())) {
826+
matchedMapping = new MatchedMapping(i, mapping);
827+
getterType = Type.getType(mapping.getJavaType());
828+
}
829+
}
830+
createJavaGetter(classWriter, preparedClassInfo, matchedMapping, attributeName, attributeType, getterType, signature,
831+
typeHint);
832+
createJavaSetter(classWriter, preparedClassInfo, matchedMapping, attributeName, attributeType, getterType, signature,
833+
typeHint);
795834
}
796835

797-
private static void createJavaGetter(ClassWriter classWriter, PreparedClassInfo preparedClassInfo, String attributeName,
836+
private static void createJavaGetter(ClassWriter classWriter, PreparedClassInfo preparedClassInfo,
837+
MatchedMapping matchedMapping, String attributeName,
798838
Type attributeType, Type getterType, String signature, TypeHint typeHint) {
799839
var getterName = "get" + attributeName.substring(0, 1).toUpperCase() + attributeName.substring(1);
800840
if (signature != null && Objects.equals(attributeType, getterType)) {
@@ -826,14 +866,30 @@ private static void createJavaGetter(ClassWriter classWriter, PreparedClassInfo
826866
// If branch is not taken, stack is null
827867
}
828868
if (!Objects.equals(attributeType, getterType)) {
869+
if (matchedMapping != null) {
870+
getterVisitor.visitInsn(Opcodes.DUP);
871+
getterVisitor.visitInsn(Opcodes.ACONST_NULL);
872+
Label skipMapping = new Label();
873+
getterVisitor.visitJumpInsn(Opcodes.IF_ACMPEQ, skipMapping);
874+
getterVisitor.visitFieldInsn(Opcodes.GETSTATIC, preparedClassInfo.classInternalName,
875+
PYTHON_JAVA_TYPE_MAPPING_PREFIX + matchedMapping.index,
876+
Type.getDescriptor(PythonJavaTypeMapping.class));
877+
getterVisitor.visitInsn(Opcodes.SWAP);
878+
getterVisitor.visitMethodInsn(Opcodes.INVOKEINTERFACE,
879+
Type.getInternalName(PythonJavaTypeMapping.class), "toJavaObject",
880+
Type.getMethodDescriptor(Type.getType(Object.class), Type.getType(Object.class)),
881+
true);
882+
getterVisitor.visitLabel(skipMapping);
883+
}
829884
getterVisitor.visitTypeInsn(Opcodes.CHECKCAST, getterType.getInternalName());
830885
}
831886
getterVisitor.visitInsn(Opcodes.ARETURN);
832887
getterVisitor.visitMaxs(maxStack, 0);
833888
getterVisitor.visitEnd();
834889
}
835890

836-
private static void createJavaSetter(ClassWriter classWriter, PreparedClassInfo preparedClassInfo, String attributeName,
891+
private static void createJavaSetter(ClassWriter classWriter, PreparedClassInfo preparedClassInfo,
892+
MatchedMapping matchedMapping, String attributeName,
837893
Type attributeType, Type setterType, String signature, TypeHint typeHint) {
838894
var setterName = "set" + attributeName.substring(0, 1).toUpperCase() + attributeName.substring(1);
839895
if (signature != null && Objects.equals(attributeType, setterType)) {
@@ -861,6 +917,21 @@ private static void createJavaSetter(ClassWriter classWriter, PreparedClassInfo
861917
// If branch is not taken, stack is None
862918
}
863919
if (!Objects.equals(attributeType, setterType)) {
920+
if (matchedMapping != null) {
921+
setterVisitor.visitVarInsn(Opcodes.ALOAD, 1);
922+
setterVisitor.visitInsn(Opcodes.ACONST_NULL);
923+
Label skipMapping = new Label();
924+
setterVisitor.visitJumpInsn(Opcodes.IF_ACMPEQ, skipMapping);
925+
setterVisitor.visitFieldInsn(Opcodes.GETSTATIC, preparedClassInfo.classInternalName,
926+
PYTHON_JAVA_TYPE_MAPPING_PREFIX + matchedMapping.index,
927+
Type.getDescriptor(PythonJavaTypeMapping.class));
928+
setterVisitor.visitInsn(Opcodes.SWAP);
929+
setterVisitor.visitMethodInsn(Opcodes.INVOKEINTERFACE,
930+
Type.getInternalName(PythonJavaTypeMapping.class), "toPythonObject",
931+
Type.getMethodDescriptor(Type.getType(Object.class), Type.getType(Object.class)),
932+
true);
933+
setterVisitor.visitLabel(skipMapping);
934+
}
864935
setterVisitor.visitTypeInsn(Opcodes.CHECKCAST, attributeType.getInternalName());
865936
}
866937
setterVisitor.visitFieldInsn(Opcodes.PUTFIELD, preparedClassInfo.classInternalName,

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/PythonCompiledClass.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import java.util.List;
44
import java.util.Map;
5+
import java.util.Set;
56

7+
import ai.timefold.jpyinterpreter.types.PythonJavaTypeMapping;
68
import ai.timefold.jpyinterpreter.types.PythonLikeType;
79
import ai.timefold.jpyinterpreter.types.wrappers.CPythonType;
810
import ai.timefold.jpyinterpreter.types.wrappers.OpaquePythonReference;
@@ -41,6 +43,11 @@ public class PythonCompiledClass {
4143
*/
4244
public List<Class<?>> javaInterfaces;
4345

46+
/**
47+
* Mapping from Python types to Java types
48+
*/
49+
public List<PythonJavaTypeMapping<?, ?>> pythonJavaTypeMappings;
50+
4451
/**
4552
* The binary type of this PythonCompiledClass;
4653
* typically {@link CPythonType}. Used when methods
@@ -62,6 +69,11 @@ public class PythonCompiledClass {
6269
*/
6370
public Map<String, OpaquePythonReference> staticAttributeNameToClassInstance;
6471

72+
/**
73+
* Contains static attributes that have get/set descriptors
74+
*/
75+
public Set<String> staticAttributeDescriptorNames;
76+
6577
public String getGeneratedClassBaseName() {
6678
return getGeneratedClassBaseName(module, qualifiedName);
6779
}

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/PythonLikeObject.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ public interface PythonLikeObject {
125125
return PythonBoolean.valueOf(!Objects.equals(this, other));
126126
}
127127

128-
default PythonLikeObject $method$__str__() {
129-
return PythonString.valueOf(this.toString());
128+
default PythonString $method$__str__() {
129+
return PythonString.valueOf(this.getClass().getSimpleName() + "@" + System.identityHashCode(this));
130130
}
131131

132132
default PythonLikeObject $method$__repr__() {

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/AbstractPythonLikeObject.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,9 @@ public AbstractPythonLikeObject(PythonLikeType __type__, Map<String, PythonLikeO
4949
public void setAttribute(String attributeName, PythonLikeObject value) {
5050
__dir__.put(attributeName, value);
5151
}
52+
53+
@Override
54+
public String toString() {
55+
return $method$__str__().toString();
56+
}
5257
}

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/Ellipsis.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ private Ellipsis() {
1818
super(ELLIPSIS_TYPE);
1919
}
2020

21+
@Override
22+
public PythonString $method$__str__() {
23+
return PythonString.valueOf(toString());
24+
}
25+
2126
@Override
2227
public String toString() {
2328
return "NotImplemented";

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/NotImplemented.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ private NotImplemented() {
1717
super(NOT_IMPLEMENTED_TYPE);
1818
}
1919

20+
@Override
21+
public PythonString $method$__str__() {
22+
return PythonString.valueOf(toString());
23+
}
24+
2025
@Override
2126
public String toString() {
2227
return "NotImplemented";

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/PythonByteArray.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1914,6 +1914,11 @@ public PythonString repr() {
19141914
return asString();
19151915
}
19161916

1917+
@Override
1918+
public PythonString $method$__str__() {
1919+
return PythonString.valueOf(toString());
1920+
}
1921+
19171922
@Override
19181923
public String toString() {
19191924
StringBuilder out = new StringBuilder(valueBuffer.limit());

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/PythonBytes.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,6 +1686,11 @@ public PythonString repr() {
16861686
return asString();
16871687
}
16881688

1689+
@Override
1690+
public PythonString $method$__str__() {
1691+
return PythonString.valueOf(toString());
1692+
}
1693+
16891694
@Override
16901695
public String toString() {
16911696
boolean hasSingleQuotes = false;

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/PythonJavaTypeMapping.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package ai.timefold.jpyinterpreter.types;
2+
3+
public interface PythonJavaTypeMapping<PythonType_, JavaType_> {
4+
PythonLikeType getPythonType();
5+
6+
Class<? extends JavaType_> getJavaType();
7+
8+
PythonType_ toPythonObject(JavaType_ javaObject);
9+
10+
JavaType_ toJavaObject(PythonType_ pythonObject);
11+
}

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/PythonLikeType.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,11 @@ public List<PythonLikeType> getParentList() {
598598
return PARENT_TYPES;
599599
}
600600

601+
@Override
602+
public PythonString $method$__str__() {
603+
return PythonString.valueOf(toString());
604+
}
605+
601606
@Override
602607
public String toString() {
603608
return "<class " + TYPE_NAME + ">";

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/PythonNone.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ private PythonNone() {
3030
super(BuiltinTypes.NONE_TYPE);
3131
}
3232

33+
@Override
34+
public PythonString $method$__str__() {
35+
return PythonString.valueOf(toString());
36+
}
37+
3338
@Override
3439
public String toString() {
3540
return "None";

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/PythonString.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,6 +1477,11 @@ public PythonString asString() {
14771477
return this;
14781478
}
14791479

1480+
@Override
1481+
public PythonString $method$__str__() {
1482+
return this;
1483+
}
1484+
14801485
@Override
14811486
public String toString() {
14821487
return value;

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/collections/PythonLikeDict.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,11 @@ public int hashCode() {
402402
return Objects.hash(delegate);
403403
}
404404

405+
@Override
406+
public PythonString $method$__str__() {
407+
return PythonString.valueOf(toString());
408+
}
409+
405410
@Override
406411
public String toString() {
407412
return delegate.toString();

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/collections/PythonLikeList.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,11 @@ public List<T> subList(int i, int i1) {
558558
return delegate.subList(i, i1);
559559
}
560560

561+
@Override
562+
public PythonString $method$__str__() {
563+
return PythonString.valueOf(toString());
564+
}
565+
561566
@Override
562567
public String toString() {
563568
StringBuilder out = new StringBuilder();

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/collections/PythonLikeTuple.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import ai.timefold.jpyinterpreter.types.PythonLikeComparable;
2121
import ai.timefold.jpyinterpreter.types.PythonLikeType;
2222
import ai.timefold.jpyinterpreter.types.PythonSlice;
23+
import ai.timefold.jpyinterpreter.types.PythonString;
2324
import ai.timefold.jpyinterpreter.types.errors.TypeError;
2425
import ai.timefold.jpyinterpreter.types.errors.ValueError;
2526
import ai.timefold.jpyinterpreter.types.errors.lookup.IndexError;
@@ -444,6 +445,11 @@ public int hashCode() {
444445
return PythonInteger.valueOf(hashCode());
445446
}
446447

448+
@Override
449+
public PythonString $method$__str__() {
450+
return PythonString.valueOf(toString());
451+
}
452+
447453
@Override
448454
public String toString() {
449455
return delegate.toString();

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/datetime/PythonTime.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,9 @@ public PythonString isoformat(PythonString formatSpec) {
328328
return PythonString.valueOf(result);
329329
}
330330

331-
public PythonString toPythonString() {
332-
return new PythonString(localTime.toString());
331+
@Override
332+
public PythonString $method$__str__() {
333+
return PythonString.valueOf(toString());
333334
}
334335

335336
@Override

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/datetime/PythonTimeDelta.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,11 @@ public PythonBoolean isZero() {
339339
return PythonBoolean.valueOf(duration.isZero());
340340
}
341341

342+
@Override
343+
public PythonString $method$__str__() {
344+
return PythonString.valueOf(toString());
345+
}
346+
342347
@Override
343348
public String toString() {
344349
StringBuilder out = new StringBuilder();

‎jpyinterpreter/src/main/java/ai/timefold/jpyinterpreter/types/numeric/PythonBoolean.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ public static PythonBoolean valueOf(boolean result) {
123123
return BuiltinTypes.BOOLEAN_TYPE;
124124
}
125125

126+
@Override
127+
public PythonString $method$__str__() {
128+
return PythonString.valueOf(toString());
129+
}
130+
126131
@Override
127132
public String toString() {
128133
if (this == TRUE) {

0 commit comments

Comments
 (0)