Skip to content

Conversation

@SoongNoonien
Copy link
Member

In my mission to completing Nemocas/AbstractAlgebra.jl#2182 I discovered that the experimental ActionPolyRing uses the universal polynomial rings. With this AbstractAlgebra PR the meaning of base_ring for universal polynomial rings will change and coefficient_ring should be used instead. Perhaps coefficient_ring should be implemented for the ActionPolyRing as well? (See: #1505 and Nemocas/AbstractAlgebra.jl#1207)

@SirToby25 I'm pinging you as you are mentioned in the docs of the ActionPolyRing.

@codecov
Copy link

codecov bot commented Oct 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.99%. Comparing base (b2f5345) to head (069ac33).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5427      +/-   ##
==========================================
- Coverage   83.99%   83.99%   -0.01%     
==========================================
  Files         721      721              
  Lines       98479    98479              
==========================================
- Hits        82718    82716       -2     
- Misses      15761    15763       +2     
Files with missing lines Coverage Δ
experimental/ActionPolyRing/src/Content.jl 97.99% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

@joschmitt joschmitt added experimental Only changes experimental parts of the code release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Oct 8, 2025
@lgoettgens
Copy link
Member

Perhaps coefficient_ring should be implemented for the ActionPolyRing as well?

I would agree with you. @SirToby25 could you take care of this?

@SirToby25
Copy link
Collaborator

Yes, I will do that. I will also change the functionality of base_ring in accordance with Nemocas/AbstractAlgebra.jl#2182, so that it replaces __upr

@lgoettgens
Copy link
Member

Yes, I will do that. I will also change the functionality of base_ring in accordance with Nemocas/AbstractAlgebra.jl#2182, so that it replaces __upr

Actually, you cannot do that right now as this will need a breaking AA release first.
Thus, it would be best to split your work into two parts: The first one can be merged right now and just adds coefficient_ring. The second one can be merged right after the AA release (this can then replace the __upr thing).

@SirToby25 SirToby25 mentioned this pull request Oct 8, 2025
@fingolfin fingolfin merged commit b3a21f3 into oscar-system:master Oct 8, 2025
37 of 40 checks passed
@SoongNoonien SoongNoonien deleted the action_poly_coefficient branch November 3, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experimental Only changes experimental parts of the code release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants