Skip to content

Conversation

@fingolfin
Copy link
Member

@fingolfin fingolfin commented Dec 3, 2025

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
Copy link

codecov bot commented Dec 19, 2025

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

3 participants