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

have float factory take an optional :precision value #261

Open
kellyredding opened this issue Apr 21, 2016 · 5 comments
Open

have float factory take an optional :precision value #261

kellyredding opened this issue Apr 21, 2016 · 5 comments
Labels
maybe unsure if will do this or not

Comments

@kellyredding
Copy link
Member

This would allow generating floats with a specific precision.

@kellyredding
Copy link
Member Author

kellyredding commented May 12, 2018

@jcredding do you remember why I put this issue in? I can't right now and stupidly didn't put any context into the issue or my dev notes.

I have something working for this but am not in love with it. Also, these are floating point numbers so exact values are a lie. Just b/c a value is 12.456 doesn't mean you can't express that same number in a higher precision and end up with 12.45599912345322.

My implementation looks like this:

Factory.float(13)                  #=> 12.6381341843036762
Factory.float(13, :precision => 2) #=> 10.12
Factory.float(:precision => 2)     #=> 95.79

I don't love the method signature but I have to account for both optionally passing a max value and optionally passing a precision value. Couple that with the float values are a lie bit above and I'm ver lukewarm on this (unless we have a good use-case for this).

Thanks for any help context.

@kellyredding
Copy link
Member Author

@jcredding also the implementation looks something like this:

sprintf("%0.#{opts[:precision] || 16}f", (max || 100) * rand).to_f

(16 is the default precision used by rand, btw). Figured you should see this and weigh it when deciding)

@jcredding
Copy link
Member

@kellyredding - I'm fine with adding this though I don't know of a good reason we'd need it currently. Maybe with much-decimal and only wanting to use random floats based on the precision used with much-decimal? I generally prefer testing with full random values (so don't set a precision) but other's might prefer having more "realistic" (it's unlikely someone enters a float with 16 digits) values.

As far as the implementation, I would have done the integer trick where you multiple it by 10^precision, convert to an integer, then divide by the 10^precision but that's more because I forget about sprintf. I don't have an issue with that implementation; no idea what's more performant but the sprintf seems like less work.

@kellyredding
Copy link
Member Author

@jcredding I think I'm going to hold off on this for now. I just don't feel good adding it w/o a good use case. I'll keep this issue around with our notes for future reference if a use case does come up though. Cool?

@kellyredding kellyredding added the maybe unsure if will do this or not label May 13, 2018
@kellyredding
Copy link
Member Author

(notes from last time I played with this)

      def self.float(*args)
        opts, max = [
          args.last.kind_of?(::Hash) ? args.pop : {},
          args.last
        ]
        sprintf("%0.#{opts[:precision] || 16}f", (max || 100) * rand).to_f
      end

    should "allow passing a precision value using `float`" do
      skip "TODO: test precision w/ and w/o max value"
    end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maybe unsure if will do this or not
Projects
None yet
Development

No branches or pull requests

2 participants