-
Notifications
You must be signed in to change notification settings - Fork 63
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
fix: make compiler happy #1611
fix: make compiler happy #1611
Conversation
@@ -66,7 +66,7 @@ ring. | |||
""" | |||
function gens(a::MPolyRing{T}) where {T <: RingElement} | |||
n = a.num_vars | |||
return [gen(a, i, Val{a.ord}) for i in 1:n] | |||
return elem_type(a)[gen(a, i, Val{a.ord})::elem_type(a) for i in 1:n] |
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.
return elem_type(a)[gen(a, i, Val{a.ord})::elem_type(a) for i in 1:n] | |
return elem_type(a)[gen(a, i, Val(a.ord))::elem_type(a) for i in 1:n] |
Val
objects are supposed to be created with ()
, the {}
is their type
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.
Yes, but the methods expect their type. It is unstable anyway.
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.
Ok. I don't really care.
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.
Val{a.ord}
and Val(a.ord)
do different things and unless the suggestion was to rewrite everyhing, I don't understand the comment.
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.
please just ignore the comment
If re-running the in validations job enough times makes it green, this is fine from my pov |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1611 +/- ##
=======================================
Coverage 86.96% 86.96%
=======================================
Files 114 114
Lines 29642 29642
=======================================
Hits 25778 25778
Misses 3864 3864 ☔ View full report in Codecov by Sentry. |
No description provided.