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.

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 limit_denominator method with tests #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

msalib
Copy link

@msalib msalib commented Apr 7, 2022

Python's fractions module in the standard library implements a rational type and provides a really handy limit_denominator method. I've implemented it here along with some tests.

This PR should address #99 .

@cmpute
Copy link

cmpute commented Apr 8, 2022

FWIW, I would suggest creating a uniform method/trait to approximate numbers (as I proposed in #100), which should work for both float to rational and rational to rational conversions.

@cuviper
Copy link
Member

cuviper commented Apr 26, 2022

I appreciate that you cited your source, but I'm also uncomfortable with the license implications there. I think language translations do still generally count as derived works. I'm not really sure what we need to do about that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants