Skip to content
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.

Sign up for GitHub

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

Jump to bottom

"rationalize" a rational #43369

Closed
ParadaCarleton opened this issue Dec 8, 2021 · 8 comments · Fixed by #43427
Closed

"rationalize" a rational #43369

ParadaCarleton opened this issue Dec 8, 2021 · 8 comments · Fixed by #43427
Labels
good first issue Indicates a good issue for first-time contributors to Julia

Comments

@ParadaCarleton
Copy link
Contributor

It might be useful to have rationalize(x) return x if x is already rational, to allow generic code that takes both reals and rationals as inputs.
@oscardssmith oscardssmith added the good first issue Indicates a good issue for first-time contributors to Julia label Dec 8, 2021
@jlazar17 jlazar17 mentioned this issue Dec 8, 2021
Closed
@ayushsengupta8
Copy link

Hello,
I am completely new to open source projects and would like to try to solve this issue.
I can use a convert() function to rationalize(x), however, it would be helpful if I know in which folder, in which file do I have to change/add the code. @logankilpatrick

@fredrikekre
Copy link
Member

As you can see by the link just above your comment someone has already started to work on this.

@ayushsengupta8
Copy link

@fredrikekre Oh! Thanks...and sorry for the lame comment

@tantheta01 tantheta01 mentioned this issue Dec 15, 2021
Merged
@ndinsmore
Copy link
Contributor

@dkarrasch I was just looking at this issue and it looks like you meant for it to be closed by #43427

@vmpyr
Copy link
Contributor

vmpyr commented Jan 31, 2023

This issue seems to have been resolved. I think it should be closed now

@vtjnash
Copy link
Member

vtjnash commented Jan 31, 2023

Still fails. (waiting for #43427 to be merged)

julia> rationalize(rationalize(1.0))
ERROR: MethodError: no method matching rationalize(::Rational{Int64})

@vmpyr
Copy link
Contributor

vmpyr commented Jan 31, 2023

I am sorry, I misinterpreted something. My bad. Is that PR still WIP?

@ParadaCarleton
Copy link
Contributor Author

I am sorry, I misinterpreted something. My bad. Is that PR still WIP?

I don't think it is, so much as it's finished but hasn't been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Indicates a good issue for first-time contributors to Julia
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add method to rationalize Rational tantheta01/julia
7 participants
@vtjnash @fredrikekre @oscardssmith @ndinsmore @ayushsengupta8 @ParadaCarleton @vmpyr