TimefoldAI / timefold-solver-python

Timefold Solver is an AI constraint solver for Python to optimize the Vehicle Routing Problem, Employee Rostering, Maintenance Scheduling, Task Assignment, School Timetabling, Cloud Optimization, Conference Scheduling, Job Shop Scheduling, Bin Packing and many more planning problems.
https://timefold.ai
Apache License 2.0
27 stars 3 forks source link

feat: Improve ScoreAnalysis debug information #105

Closed zepfred closed 2 days ago

zepfred commented 5 days ago

This pull request adds a summary logic to the ScoreAnalysis and ConstraintAnalysis classes. Additionally, it synchronizes the API of the Python _score_analysis.py with the Java classes.

zepfred commented 5 days ago

@triceo I created a new pull request because the branch was renamed to run the tests correctly. Your comments from #102 have been addressed.

zepfred commented 2 days ago

@Christopher-Chianelli @triceo, do we all agree to use the summarize as a method?

def assert_score_analysis_summary(score_analysis: ScoreAnalysis):
    summary = score_analysis.summarize()
    ...
    match = score_analysis.constraint_analyses[0]
    match_summary = match.summarize()
    ...
triceo commented 2 days ago

@Christopher-Chianelli My rationale for summarize is that this isn't actually an accessor for a property, it is not a constant - in that case, arguably summary would be correct. But summarize is a method that computes a result when it is called, which I think warrants the different name. IMO it has nothing to do with Python or Java.

Are you telling me that in Python, they don't actually make a distinction between methods and properties, even though they clearly have those constructs?

triceo commented 2 days ago

I give up. This is not worth the time we already spent on it.

@zepfred, please, change it back to summary and make it a property. I'm sorry for this.

triceo commented 2 days ago

I was deleting one of my comments and accidentally deleted yours, @Christopher-Chianelli. My apologies, was not intentional.

Christopher-Chianelli commented 2 days ago

No worries @triceo

sonarcloud[bot] commented 2 days ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

Christopher-Chianelli commented 2 days ago

@zepfred Please state clearly when a PR depends on another PR; in particular, this PR shouldn't have been merged before https://github.com/TimefoldAI/timefold-solver/pull/923

zepfred commented 2 days ago

@zepfred Please state clearly when a PR depends on another PR; in particular, this PR shouldn't have been merged before TimefoldAI/timefold-solver#923

Understood!