fix: Use Python object for Score, handle descriptor correctly, do not copy parent fields

[Christopher-Chianelli]

• 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). For Python 3.10, we copied the inspect.getmembers_static code.

• 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.

[sonarcloud[bot]]

Quality Gate passed

Issues
48 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[zepfred]

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.

There are some nice Java tests validating the conversion between Python and Java objects. However, I couldn't find any Python code using or testing the new score structure. I recommend adding some domain/tests using medium and bendable scores.

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.

There are some new tests, but I'm not entirely sure they cover all the behavior added by this change. Am I wrong?

Made toString() call $method$__str__(), and change the return
type of $method$__str__() to PythonString so overrides work
correctly.

I noticed that you didn't update all Python types, such as 'PythonLikeList'. Is there any specific reason for that? If not, please double-check to ensure all the required types are updated.

[Christopher-Chianelli]

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.

There are some nice Java tests validating the conversion between Python and Java objects. However, I couldn't find any Python code using or testing the new score structure. I recommend adding some domain/tests using medium and bendable scores.

That because all the existing tests (which previously uses the Java score) uses the Python score and weren't change

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.

There are some new tests, but I'm not entirely sure they cover all the behavior added by this change. Am I wrong?

The Score python classes cover this behaviour.

Made toString() call $method$__str__(), and change the return
type of $method$__str__() to PythonString so overrides work
correctly.

I noticed that you didn't update all Python types, such as 'PythonLikeList'. Is there any specific reason for that? If not, please double-check to ensure all the required types are updated.

Generally speaking, only the Python type that overridden toString would need to be updated.

[zepfred]

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.

There are some nice Java tests validating the conversion between Python and Java objects. However, I couldn't find any Python code using or testing the new score structure. I recommend adding some domain/tests using medium and bendable scores.

That because all the existing tests (which previously uses the Java score) uses the Python score and weren't change

Okay, I still believe that creating Python classes using the new structure is a good idea, similar to what we did in tests/test_domain.py. Regardless, this is not a blocker if you ensured testing the new score classes locally.

Made toString() call $method$__str__(), and change the return
type of $method$__str__() to PythonString so overrides work
correctly.

I noticed that you didn't update all Python types, such as 'PythonLikeList'. Is there any specific reason for that? If not, please double-check to ensure all the required types are updated.

Generally speaking, only the Python type that overridden toString would need to be updated.

That means the non-updated types will continue using the "incorrect" logic. In this case, we should keep the new logic consistent across all types.

[Christopher-Chianelli]

That means the non-updated types will continue using the "incorrect" logic. In this case, we should keep the new logic consistent across all types.

No, the logic for things without toString would be the same.