-
Notifications
You must be signed in to change notification settings - Fork 23
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
convert Rational to fmpq #62
base: master
Are you sure you want to change the base?
Conversation
src/flint/fmpq.jl
Outdated
@@ -713,6 +713,8 @@ convert(::Type{fmpq}, a::Integer) = fmpq(a) | |||
|
|||
convert(::Type{fmpq}, a::fmpz) = fmpq(a) | |||
|
|||
Base.convert(::Type{Nemo.fmpq}, x::Rational) = Nemo.fmpq(x.num, x.den) |
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.
Again, I don't believe the qualifiers are needed here.
Can I fix the qualifiers here, or do I need to make a new pull request? |
As I guess you figured out, you only have to push new commits to the branch you made. They show up here automatically. |
@@ -713,6 +713,8 @@ convert(::Type{fmpq}, a::Integer) = fmpq(a) | |||
|
|||
convert(::Type{fmpq}, a::fmpz) = fmpq(a) | |||
|
|||
Base.convert(::Type{fmpq}, x::Rational) = fmpq(x.num, x.den) |
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.
Does convert need the Base qualifier? We don't seem to have it on the previous line.
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.
I'm not sure. I did not find any import Base.convert
.
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.
Our Base imports are in Nemo/src/Nemo.jl
On 13 September 2016 at 01:12, Jorge Fernández de Cossío Díaz <
[email protected]> wrote:
In src/flint/fmpq.jl
#62 (comment):@@ -713,6 +713,8 @@ convert(::Type{fmpq}, a::Integer) = fmpq(a)
convert(::Type{fmpq}, a::fmpz) = fmpq(a)
+Base.convert(::Type{fmpq}, x::Rational) = fmpq(x.num, x.den)
I'm not sure. I did not find any import Base.convert.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/wbhart/Nemo.jl/pull/62/files/25c9cb65f332a4f014fa3a04a8938d85ab7198e7#r78471874,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOzpnQD48yM5YJdsV1jhffxQYQicgFiks5qpdxagaJpZM4J6u5P
.
No description provided.