Re: [boost] [rational] Can the community maintenance team and/or rational users please review some pull requests?

Subject: Re: [boost] [rational] Can the community maintenance team and/or rational users please review some pull requests?
From: Marc Glisse (marc.glisse_at_[hidden])
Date: 2015-03-30 13:16:17

On Mon, 30 Mar 2015, John Maddock wrote:

> I have a bunch of PR's against Rational, which of course is basically 
> unmaintained.  I'd be grateful if a second pair of eyes (at least) could take 
> a look before I start agitating for someone to merge them ;)
>
> Each of the four, builds on the previous one(s).
>
> Three are basically trivial:
>
> https://github.com/boostorg/rational/pull/1/files
> https://github.com/boostorg/rational/pull/2/files
> https://github.com/boostorg/rational/pull/3/files
>
> The final one is much more complex, and disables "accidental" conversion from 
> float to rational (which otherwise proceeds via truncation to integer first).
>
> As you can see from the commit history, I've had quite a few attempts at 
> getting the final one correct.  I believe it is now, doesn't break anything 
> in Boost except, as noted in the discussion, Boost.Geometry, which relies on 
> an unsafe float to rational conversion - Geometry's authors are aware of the 
> issue here: 
> https://github.com/boostorg/geometry/commit/c992eb61e8ef9e90fdae403ac6ff1eea12297291.
>
> So.... I'm interested in both eyeball-look-throughs from seasoned Boosters, 
> but also feedback from Rational's users, and verification that it doesn't 
> break end user code - other than unsafe conversions of course.  If you just 
> want the updated header to test, you can grab it from here: 
> https://raw.githubusercontent.com/jzmaddock/rational/no-float-convert/include/boost/rational.hpp

If I understand correctly, cpp_rational is constructible from double but 
rational isn't? It is documented, so it's ok, but it may still confuse 
some users.

(I have never used either, but I use some alternatives, and I am planning 
to use cpp_rational soon, and construction from double is necessary for my 
use (hence ticket 10082, thanks again for fixing that so quickly))

-- 
Marc Glisse