Skip to content

Remove ApproxEq and assert_approx_eq! #11402

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

Merged
merged 1 commit into from
Jan 9, 2014

Conversation

brendanzab
Copy link
Member

This trait seems to stray too far from the mandate of a standard library as implementations may vary between use cases. Third party libraries should implement their own if they need something like it.

This closes #5316.

r? @alexcrichton, @pcwalton

@brson
Copy link
Contributor

brson commented Jan 8, 2014

I'm all for trimming fat, and this feature is kind of borderline to me. It's not uncommon I think for testing frameworks to provide such a function, but it is mostly only ever going to be used for testing floats, and open coding a comparison with epsilon is not hard. I wouldn't be sad to see it go.

@vadimcn
Copy link
Contributor

vadimcn commented Jan 8, 2014

Approximate equality is sometimes useful in numerical libraries too. In fact, that's the only kind of equality one should be using on floats! And it's surprisingly tricky to implement correctly.
Maybe the macros had to go, but the trait should have stayed, IMHO. I don't see why is it different from the rest of numeric functions.

@alexcrichton
Copy link
Member

I certainly agree that this is useful, but keep in mind that libstd is linked to all rust programs in existence by default. The threshold for putting this in libstd should be "will the majority of all program in existence want this functionality" rather than "this is very useful functionality for this subset of programs".

I think we're all in agreement that we'd like a high quality numerics library, and this would most certainly have a home in such a library.

@vadimcn
Copy link
Contributor

vadimcn commented Jan 8, 2014

I dunno, it seems strange that for example abs_sub() and next_after() do make the cut, but approximate comparison doesn't... Is the idea to only re-export stuff that libm provides?

@alexcrichton
Copy link
Member

This is not the last thing that is going to get removed from libstd, this is just the beginning.

@brendanzab
Copy link
Member Author

@vadimcn I'm soon going to begin work on libnum, which should be the goto library for this kind of stuff. Hopefully it will then be plonked into the main rust repo.

@brendanzab
Copy link
Member Author

@alexcrichton Weird, do you know why this failed?

This trait seems to stray too far from the mandate of a standard library as implementations may vary between use cases.
@brendanzab
Copy link
Member Author

Fixed the errors. r?

bors added a commit that referenced this pull request Jan 9, 2014
This trait seems to stray too far from the mandate of a standard library as implementations may vary between use cases. Third party libraries should implement their own if they need something like it.

This closes #5316.

r? @alexcrichton, @pcwalton
@bors bors closed this Jan 9, 2014
@bors bors merged commit ceea85a into rust-lang:master Jan 9, 2014
@brendanzab brendanzab deleted the remove-approx branch January 9, 2014 15:38
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.

Unexpected result of fuzzy_eq() on large number
5 participants