Skip to content

Remove legacy polymake interfacing code - #136

Merged
grahamknockillaree merged 1 commit into
masterfrom
mh/remove-legacy-polymake-code
Dec 19, 2025
Merged

grahamknockillaree merged 1 commit into
masterfrom
mh/remove-legacy-polymake-code

Conversation

@fingolfin

@fingolfin fingolfin commented Dec 3, 2025 •

Copy link
Copy Markdown
Member

Instead rely on the polymaking package.

It works locally for me but fails in CI here, so clearly needs some more work.

@fingolfin
fingolfin marked this pull request as draft December 3, 2025 16:46
@fingolfin
fingolfin force-pushed the mh/remove-legacy-polymake-code branch from 8cf89eb to de9fb6c Compare December 19, 2025 13:37
@fingolfin
fingolfin force-pushed the mh/remove-legacy-polymake-code branch 2 times, most recently from ed24b15 to 07f0e3b Compare December 19, 2025 17:07
@codecov

codecov Bot commented Dec 19, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 56.75676% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.73%. Comparing base (6c3d924) to head (da03ffa).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
lib/Polymake/orbitPoly.gi 16.66% 25 Missing ⚠️
lib/Polymake/polyGens.gi 84.09% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
- Coverage   44.81%   44.73%   -0.09%     
==========================================
  Files         345      345              
  Lines       64893    64760     -133     
==========================================
- Hits        29085    28971     -114     
+ Misses      35808    35789      -19     
Files with missing lines Coverage Δ
lib/externalSoftware.gap 85.00% <ø> (-1.85%) ⬇️
lib/Polymake/polyGens.gi 78.94% <84.09%> (-10.09%) ⬇️
lib/Polymake/orbitPoly.gi 46.80% <16.66%> (-3.78%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fingolfin
fingolfin marked this pull request as ready for review December 19, 2025 17:13
Instead rely on the polymaking package
@fingolfin
fingolfin force-pushed the mh/remove-legacy-polymake-code branch from 07f0e3b to da03ffa Compare December 19, 2025 17:16
@fingolfin

Copy link
Copy Markdown
Member Author

Yay, this works now, too! I had made a rather stupid mistake right at the start of the function where I reordered a loop thinking "this is equivalent but more elegant". When I looked at it again, I immediately realized that it was of course not equivalent... sigh Anyway, this is passing all tests now :-)

@grahamknockillaree
grahamknockillaree merged commit 7d6f890 into master Dec 19, 2025
6 of 7 checks passed
@grahamknockillaree
grahamknockillaree deleted the mh/remove-legacy-polymake-code branch December 19, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants