-
Notifications
You must be signed in to change notification settings - Fork 126
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
Oscar Singular coercion ambiguities #976
Comments
method 2. Please relieve us of this R(x) syntax. |
As we have all experienced, the conversion between Singular rings and the Oscar side can be a bit error prone. Note that the same problem was exhibited by the GAP <-> Oscar conversions, which was solved by introducing the wonderful function function iso_oscar_gap(R::Ring) which returns a map object with proper domain and codomain realising this isomorphism. I propose that we do the same here and introduce function iso_oscar_singular(R::MPolyRing) which returns a map object that one can apply etc. What do you think @tthsqe12? Since @ederc is also a fan of P.S.: We might also need |
Having discussed this with @thofma I think this might be a good and more secure concept handling conversions. |
so |
We probably will remove |
ok |
What's the status of this? The original issue seems to be resolved, but I see no trace of |
Not resolved. We just fixed the cases that popped up. |
I included a new/still failing in example in the original post. |
julia is right! This is ambiguous and the fix works only for
RationalFunctionField
. But the problem persists for any ring typeS
which has a method of the form(R::S)(x::RingElem)
.The problem is that
Oscar.jl/src/Rings/mpoly.jl
Lines 721 to 724 in af90aae
is too broad in the first argument (or too specific in the second).
I see two options:
for T in subtypes(Ring); @eval
trick. Problem is that this is not extensible, so only works for the ring types which have been defined till this point.R(x)
to do the conversion from the Singular to the Oscar side, but maybesingular_to_oscar(R, x)
.Any other ideas? @tthsqe12 @fieker?
The text was updated successfully, but these errors were encountered: