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 support for Normed construction from Rational (Fixes #157) #169

Merged
merged 1 commit into from
Jan 21, 2020

Conversation

kimikage
Copy link
Collaborator

This also fixes the overflow problem with Fixed construction from Rational. (Fixes #157)

Note the input range that Fixed allows (cf. issue #156).

This also fixes the overflow problem with `Fixed` construction from `Rational`.
@codecov-io
Copy link

Codecov Report

Merging #169 into master will decrease coverage by 0.31%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
- Coverage   88.64%   88.32%   -0.32%     
==========================================
  Files           5        5              
  Lines         370      377       +7     
==========================================
+ Hits          328      333       +5     
- Misses         42       44       +2
Impacted Files Coverage Δ
src/normed.jl 90.05% <100%> (+0.17%) ⬆️
src/fixed.jl 87.34% <60%> (-2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05fd7e7...091701b. Read the comment docs.

@kimikage
Copy link
Collaborator Author

I chose the input range just because it is looser (i.e. it has a low impact of the change).
@timholy, if it is not too much trouble, please tell me what the input range should be.

@kimikage
Copy link
Collaborator Author

Of course, anyone's opinion is fine. I'm worried about setting the specifications against the thoughts of "silent" users. However, I also think that this is not a good place for consensus building.

@kimikage
Copy link
Collaborator Author

I cannot see the negative effect unless we release it. I'm concentrating my efforts on releasing v0.8.0.

@kimikage kimikage merged commit cb29336 into JuliaMath:master Jan 21, 2020
@kimikage kimikage deleted the issue157 branch January 22, 2020 12:01
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.

Problems with conversions from Rational
3 participants