-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add some specialized methods for universal polynomials #2013
Conversation
Some other methods you should perhaps add are here: oscar-system/GenericCharacterTables.jl#203 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2013 +/- ##
==========================================
- Coverage 88.31% 88.29% -0.02%
==========================================
Files 98 99 +1
Lines 36126 36196 +70
==========================================
+ Hits 31903 31959 +56
- Misses 4223 4237 +14 ☔ View full report in Codecov by Sentry. |
60c5d74
to
bddd8cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks sensible to me. In case @lgoettgens has time and interested to have a quick look, I'll wait a bit more with merging this.
|
||
denominator(f::UniversalPolyRingElem{QQFieldElem}) = denominator(data(f)) | ||
|
||
function +(p::Generic.UnivPoly{T}, n::ZZRingElem) where {T} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't all of these here make sense for, let's say, all "scalar" types that have ad-hoc operations?
In particular, I am thinking about Integer
and T
(the coeff type), but I guess those would belong in AA instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such methods already exist for Integer
and T
in AA. But not for ZZRingElem
for obvious reasons. Which is why we add them here.
Example demonstrating the effect:
Then in
With this branch:
|
As @fingolfin mentioned in Nemocas/AbstractAlgebra.jl#1753 (comment), it would be nice to have some methods for certain universal polynomial rings. This is a first implementation of
denominator
. I think there should be some more methods that are useful. But I haven't found them yet as I currently only needdenominator
.Resolves oscar-system/GenericCharacterTables.jl#203