Skip to content

Add Puiseux polynomials#2391

Open
ollieclarke8787 wants to merge 21 commits into
Nemocas:masterfrom
ollieclarke8787:oc/PuiseuxPolynomialsFromLaurent
Open

Add Puiseux polynomials#2391
ollieclarke8787 wants to merge 21 commits into
Nemocas:masterfrom
ollieclarke8787:oc/PuiseuxPolynomialsFromLaurent

Conversation

@ollieclarke8787

@ollieclarke8787 ollieclarke8787 commented Apr 9, 2026

Copy link
Copy Markdown

Working repository for adding Puiseux Polynomials to AA, this should supersede oscar-system/Oscar.jl#5861

@joschmitt joschmitt added enhancement New feature or request release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Apr 9, 2026
@joschmitt joschmitt changed the title Add Puiseux polynomials package Add Puiseux polynomials Apr 9, 2026
@codecov

codecov Bot commented Apr 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 198 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.59%. Comparing base (3355a14) to head (d6e032a).

Files with missing lines Patch % Lines
src/PuiseuxMPoly.jl 0.00% 198 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2391      +/-   ##
==========================================
- Coverage   88.12%   87.59%   -0.53%     
==========================================
  Files         128      129       +1     
  Lines       32886    33084     +198     
==========================================
  Hits        28981    28981              
- Misses       3905     4103     +198     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joschmitt

Copy link
Copy Markdown
Collaborator

The test file needs to be included explicitly to be executed (there is no magic like in OSCAR). I assume it could fit in the list of includes in test/Rings-test.jl.

Comment thread src/PuiseuxMPoly.jl Outdated
Comment thread test/PuiseuxPolynomials-test.jl Outdated
Comment thread test/PuiseuxPolynomials-test.jl Outdated
@YueRen

YueRen commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

@lgoettgens Good point, I suggest we remove these for now and once this PR is merged and a new release of AA has happened, I will open a PR in OSCAR.

ollieclarke8787 and others added 2 commits May 27, 2026 11:51
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Removed trivial tests from the PuiseuxPolynomials test suite.
@thofma

thofma commented Jun 5, 2026

Copy link
Copy Markdown
Member

We plan to do a new release soonish. Do you guys want to get it in? Not sure what the status is here. I still see some "Oscar" references in the test code.

@YueRen

YueRen commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@ollieclarke8787 Do you have some time to finish the pull request at the beginning of next week?

@ollieclarke8787

Copy link
Copy Markdown
Author

@ollieclarke8787 Do you have some time to finish the pull request at the beginning of next week?

Definitely, I think we just need to remove the Oscar references from the tests, everything else should still be working.

@thofma thofma marked this pull request as ready for review June 8, 2026 11:14
@ollieclarke8787

Copy link
Copy Markdown
Author

Update: Most parts work as intended. Two remaining issues:

  1. base_ring_type(::Type{PuiseuxMPolyRing{T}}) where T <: RingElement = AA.Generic.LaurentMPolyRing{T} is not correct - I'm not sure what it should be so that typeof(base_ring(R)) == AA.Generic.LaurentMPolyWrapRing{T}
  2. There is an issue with divexact(f, a) where a lies in the coefficient ring, see the example in the code.

Comment thread src/PuiseuxMPoly.jl Outdated
Co-authored-by: Martin Wagner <martin.wagner.dev@gmail.com>
Comment thread src/PuiseuxMPoly.jl Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants