Skip to content
This repository was archived by the owner on Jul 17, 2024. It is now read-only.

Commit 458c5b9

Browse files
chore: Remove old wrapper generation code and use more Pythonic API (#23)
This refactor remove the old wrapper generation code (which was used to add annotations to fields) and uses the new jpyinterpreter API to add annotations to the fields instead. As a result, the following was able to be removed: - The Python specific solution cloner. - The Python specific wrappers of Comparable and List. - The methods in the timefold-solver-python-core module used to add annotations and cloning methods. - JPype auto-conversions for PythonLikeObject and Comparable Additionally, the API and annotations where refactored to be more Pythonic. For instance, ```python class A: @planning_variable(int) def get_x(): return x ``` becomes ```python class A: x: Annotated[int, PlanningVariable] ``` and ```python solver_config = \ SolverConfig().withSolutionClass(...).withEntityClasses(...) solver = solver_factory_create(solver_config).buildSolver() ``` becomes ```python solver_config = SolverConfig( solution_class=SolutionClass, entity_classes=[EntityClass], ...) solver = SolverFactory.create(solver_config).build_solver() ``` This process also uncovered several bugs in jpyinterpreter: - Method dispatchers for JavaObjectWrapper were not registered correctly - The jpyinterpreter objects do not know if they were clone, so they reuse the existing python references (since Timefold just treat it as a regular shallowly cloned field). A ConcurrentWeakIdentityHashMap is used to detect if the object was cloned and thus should create a new instance. - The ArgumentSpec field was not created for wrapped functions, which caused an issue for UUID.__init__ - Apparently UUID was self-referencing, meaning the globals map for its module had null in it since we put a None in the type dict to prevent infinite recursion - Now we create an "incomplete" type that we put inside the dict before superclasses/methods/fields are processed, and the type is filled in when the class is finally compiled. - Fix call to old method in Python code that was replaced by a BuiltinTypes field. - tuple comparision methods were missing - Added an exit handler to conftest that exits the JVM so PyTest does not hang from daemon thread (and allows us to remove the sleep call) The tests also had several typing errors which are now fixed. Additionally, no more extremely unuseful error message with generic advice; the stack trace is now preserved for non-signal exceptions (cause, Python really likes using exceptions in it bytecode; whenever you do a for-loop, that actually a `try { while True {...} } catch(StopIteration) {}` block). As a side-effect, if no best solution listener is registered, the code never returns to Python until Solving finishes (before, it returned to Python on each new best solution). To allow Timefold Solver to use the class object generated by jpyinterpreter, several changes need to be made: - Fields can now actually use generic, so List[int] become List[PythonLikeInteger] in the bytecode - Added a proxy generator for functions and classes. The proxy generator for functions create a class that just call a fixed instance of the function (since functions might have closures which are specific to an instance). The proxy generator for classes create a class that creates a delegate to the class in the constructor and delegate all calls to that delegate. - Since PythonLikeBoolean is not a boolean (since it need to support all Python operations), changed PlanningPin so it is read in the entity's pinning filter. The following two features are currently unsupported in this version: - ConstraintMatches in incremental score calculator. Requires changes to the unwrapping code so the items in the List are unwrapped. - ProblemChanges Additionally, upgraded min version to Python 3.10, since numpy no longers support Python 3.9 (https://numpy.org/neps/nep-0029-deprecation_policy.html) and the scientific python community also recommends dropping support for Python 3.9 since the start of this year (https://scientific-python.org/specs/spec-0000/).
1 parent fe31688 commit 458c5b9

File tree

94 files changed

+6308
-9769
lines changed

Some content is hidden

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

94 files changed

+6308
-9769
lines changed

.github/workflows/pull_request.yml

+47-20
Original file line numberDiff line numberDiff line change
@@ -23,51 +23,78 @@ jobs:
2323
strategy:
2424
matrix:
2525
os: [ ubuntu-latest ]
26-
java-version: [ 17 ]
26+
java-version: [ 17, 21, 22 ]
2727
fail-fast: false
2828
runs-on: ${{ matrix.os }}
2929

3030
steps:
31+
# Need to check for stale repo, since Github is not aware of the build chain and therefore doesn't automate it.
32+
- name: Checkout timefold-solver to access the scripts
33+
uses: actions/checkout@v4
34+
with:
35+
path: './timefold-solver'
36+
repository: 'TimefoldAI/timefold-solver'
37+
- name: Find the proper timefold-solver repo and branch
38+
env:
39+
CHAIN_USER: ${{ github.event.pull_request.head.repo.owner.login }}
40+
CHAIN_BRANCH: ${{ github.head_ref }}
41+
CHAIN_REPO: "timefold-solver"
42+
CHAIN_DEFAULT_BRANCH: ${{ endsWith(github.head_ref, '.x') && github.head_ref || 'main' }}
43+
shell: bash
44+
run: |
45+
./timefold-solver/.github/scripts/check_chain_repo.sh
46+
rm -rf ./timefold-solver
47+
- name: Checkout the proper timefold-solver branch
48+
uses: actions/checkout@v4
49+
with:
50+
repository: ${{ env.TARGET_CHAIN_USER }}/${{ env.TARGET_CHAIN_REPO }}
51+
ref: ${{ env.TARGET_CHAIN_BRANCH }}
52+
path: './timefold-solver'
53+
fetch-depth: 0 # Otherwise merge in the next step will fail on account of not having history.
54+
- name: Prevent stale fork of timefold-solver
55+
env:
56+
BLESSED_REPO: "timefold-solver"
57+
BLESSED_BRANCH: ${{ endsWith(github.head_ref, '.x') && github.head_ref || 'main' }}
58+
shell: bash
59+
working-directory: ./timefold-solver
60+
run: .github/scripts/prevent_stale_fork.sh
61+
3162
- name: Check out repository code
3263
uses: actions/checkout@v4
64+
with:
65+
path: './timefold-solver-python'
3366

67+
# Build and test
3468
- name: Set up JDK 17
3569
uses: actions/setup-java@v4
3670
with:
3771
java-version: ${{ matrix.java-version }}
3872
distribution: 'temurin'
3973
cache: 'maven'
40-
# Need to install both Python 3.9, Python 3.10 and Python 3.11 for tox (has to be in the same run)
41-
# Feature Request for setup action: https://github.com/actions/setup-python/issues/98
42-
- name: Python 3.9 Setup
43-
uses: actions/setup-python@v4
44-
with:
45-
python-version: '3.9'
46-
cache: 'pip'
47-
cache-dependency-path: |
48-
**/setup.py
49-
- name: Python 3.10 Setup
50-
uses: actions/setup-python@v4
51-
with:
52-
python-version: '3.10'
53-
cache: 'pip'
54-
cache-dependency-path: |
55-
**/setup.py
56-
- name: Python 3.11 Setup
74+
# Need to install all Python versions in the same run for tox
75+
- name: Python 3.9, Python 3.10, Python 3.11 Setup
5776
uses: actions/setup-python@v4
5877
with:
59-
python-version: '3.11'
78+
python-version: |
79+
3.9
80+
3.10
81+
3.11
6082
cache: 'pip'
6183
cache-dependency-path: |
6284
**/setup.py
6385
- name: Install tox
6486
run:
6587
python -m pip install --upgrade pip
6688
pip install tox pytest
89+
- name: Quickly build timefold-solver
90+
working-directory: ./timefold-solver
91+
run: mvn -B -Dquickly clean install
6792
- name: Build with Maven to install parent poms for python build
93+
working-directory: ./timefold-solver-python
6894
run: mvn -B --fail-at-end clean install
6995
- name: Run tox on timefold solver python test suite
96+
working-directory: ./timefold-solver-python
7097
run: python -m tox
7198
- name: Run tox on jpyinterpreter test suite
72-
working-directory: ./jpyinterpreter
99+
working-directory: ./timefold-solver-python/jpyinterpreter
73100
run: python -m tox

.github/workflows/sonarcloud.yml

+50-26
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,50 @@ defaults:
1919

2020
jobs:
2121
sonarcloud-analysis:
22-
strategy:
23-
matrix:
24-
os: [ ubuntu-latest ]
25-
java-version: [ 17 ] # JaCoCo segfaults Java 11 JVM but not Java 17 JVM when Python tests finish
26-
fail-fast: false
27-
runs-on: ${{ matrix.os }}
22+
runs-on: ubuntu-latest
2823

2924
steps:
25+
# Need to check for stale repo, since Github is not aware of the build chain and therefore doesn't automate it.
26+
- name: Checkout timefold-solver to access the scripts
27+
uses: actions/checkout@v4
28+
with:
29+
path: './timefold-solver'
30+
repository: 'TimefoldAI/timefold-solver'
31+
- name: Find the proper timefold-solver repo and branch
32+
env:
33+
CHAIN_USER: ${{ github.event.pull_request.head.repo.owner.login }}
34+
CHAIN_BRANCH: ${{ github.head_ref }}
35+
CHAIN_REPO: "timefold-solver"
36+
CHAIN_DEFAULT_BRANCH: ${{ endsWith(github.head_ref, '.x') && github.head_ref || 'main' }}
37+
shell: bash
38+
run: |
39+
./timefold-solver/.github/scripts/check_chain_repo.sh
40+
rm -rf ./timefold-solver
41+
- name: Checkout the proper timefold-solver branch
42+
uses: actions/checkout@v4
43+
with:
44+
repository: ${{ env.TARGET_CHAIN_USER }}/${{ env.TARGET_CHAIN_REPO }}
45+
ref: ${{ env.TARGET_CHAIN_BRANCH }}
46+
path: './timefold-solver'
47+
fetch-depth: 0 # Otherwise merge in the next step will fail on account of not having history.
48+
- name: Prevent stale fork of timefold-solver
49+
env:
50+
BLESSED_REPO: "timefold-solver"
51+
BLESSED_BRANCH: ${{ endsWith(github.head_ref, '.x') && github.head_ref || 'main' }}
52+
shell: bash
53+
working-directory: ./timefold-solver
54+
run: .github/scripts/prevent_stale_fork.sh
3055
- uses: actions/checkout@v4
3156
with:
3257
fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis
3358
ref: ${{ github.event.pull_request.head.sha }} # The GHA event will pull the main branch by default, and we must specify the PR reference version
59+
path: './timefold-solver-python'
60+
61+
# Build and test
3462
- name: Set up JDK 17
3563
uses: actions/setup-java@v4
3664
with:
37-
java-version: ${{ matrix.java-version }}
65+
java-version: 17
3866
distribution: 'temurin'
3967
- name: Cache SonarCloud packages
4068
uses: actions/cache@v4
@@ -48,43 +76,37 @@ jobs:
4876
path: ~/.m2
4977
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
5078
restore-keys: ${{ runner.os }}-m2
51-
# Need to install both Python 3.9, Python 3.10 and Python 3.11 for tox (has to be in the same run)
52-
# Feature Request for setup action: https://github.com/actions/setup-python/issues/98
53-
- name: Python 3.9 Setup
54-
uses: actions/setup-python@v4
55-
with:
56-
python-version: '3.9'
57-
cache: 'pip'
58-
cache-dependency-path: |
59-
**/setup.py
60-
- name: Python 3.10 Setup
61-
uses: actions/setup-python@v4
62-
with:
63-
python-version: '3.10'
64-
cache: 'pip'
65-
cache-dependency-path: |
66-
**/setup.py
67-
- name: Python 3.11 Setup
79+
# Need to install all Python versions in the same run for tox
80+
- name: Python 3.9, Python 3.10, Python 3.11 Setup
6881
uses: actions/setup-python@v4
6982
with:
70-
python-version: '3.11'
83+
python-version: |
84+
3.9
85+
3.10
86+
3.11
7187
cache: 'pip'
7288
cache-dependency-path: |
7389
**/setup.py
7490
- name: Install tox
7591
run:
7692
python -m pip install --upgrade pip
7793
pip install tox coverage pytest pytest-cov
94+
- name: Quickly build timefold-solver
95+
working-directory: ./timefold-solver
96+
run: mvn -B -Dquickly clean install
7897
- name: Build with Maven to measure code coverage
98+
working-directory: ./timefold-solver-python
7999
run: mvn -B --fail-at-end clean install -Prun-code-coverage
80100
- name: Get JaCoCo Agent
101+
working-directory: ./timefold-solver-python
81102
run: mvn org.apache.maven.plugins:maven-dependency-plugin:2.8:get -Dartifact=org.jacoco:org.jacoco.agent:0.8.11:jar:runtime -Ddest=target/jacocoagent.jar
82103
- name: Run tox to measure timefold solver python code coverage from Python tests
83104
continue-on-error: true # Sometimes the JVM segfaults on SUCCESSFUL tests with Java 17 (and always with Java 11)
105+
working-directory: ./timefold-solver-python
84106
run: python -m tox -- --cov=timefold --cov-report=xml:target/coverage.xml --cov-config=tox.ini --cov-branch --cov-append --jacoco-agent=./target/jacocoagent.jar
85107
- name: Run tox to measure jpyinterpreter code coverage from Python tests
86108
continue-on-error: true # Sometimes the JVM segfaults on SUCCESSFUL tests with Java 17 (and always with Java 11)
87-
working-directory: ./jpyinterpreter
109+
working-directory: ./timefold-solver-python/jpyinterpreter
88110
run: python -m tox -- --cov=jpyinterpreter --cov-report=xml:target/coverage.xml --cov-config=tox.ini --cov-branch --cov-append --jacoco-agent=../target/jacocoagent.jar --jacoco-output=../target/jacoco.exec
89111
# Because we are using JPype, and JPype add it own import hook, we need to use --import-mode=importlib in pytest
90112
# This seems to create an issue in test coverage, where it reports coverage inside the tox virtual environment,
@@ -98,8 +120,10 @@ jobs:
98120
# fix-coverage-paths.py is a Python script that does the above transformation for us and merge the two
99121
# separate coverage files into one.
100122
- name: Fix Python test coverage paths
123+
working-directory: ./timefold-solver-python
101124
run: python fix-coverage-paths.py
102125
- name: Run SonarCloud analysis
126+
working-directory: ./timefold-solver-python
103127
env:
104128
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
105129
SONARCLOUD_TOKEN: ${{ secrets.SONARCLOUD_TOKEN }}

0 commit comments

Comments
 (0)