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

TST add float_cmp crate for tests #54

Merged
merged 2 commits into from Nov 19, 2019
Merged

Conversation

jbowles
Copy link
Contributor

@jbowles jbowles commented May 7, 2019

What would you think about adding float-cmp for doing float comparisons in test?

Clippy complains about float comparisons and I've run into it with other projects...

first time I've tried out this crate to deal with float comparisons in tests.

Copy link
Owner

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jbowles ! Minor comment below, apart for that looks good.

I wonder if something similar exists for arrays (equivalent to numpy.testing.assert_allclose)..

Cargo.toml Outdated
@@ -44,3 +44,4 @@ ndarray = "0.12.1"
sprs = "0.6.3"
unicode-segmentation = "1.2.1"
hashbrown = "0.1"
float-cmp = "0.4.0" #for asserting floating point differences in test
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could put this under the [dev-dependencies] section, as it's only needed for tests?

Nit; also maybe adding a space after the # for readability if you don't mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure. Will do. I'll look around too for something like numpy.testing.assert_allclose.

@rth
Copy link
Owner

rth commented Jul 23, 2019

Actually I think the approx crate might be beter, because it will also work with ndarrays in the future rust-ndarray/ndarray#581

@rth rth marked this pull request as ready for review November 19, 2019 23:45
Copy link
Owner

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging, thanks! Will convert tests to using appox in a follow up PR, it would still probably be easier after this is merged.

@rth rth changed the title add float_cmp crate for tests TST add float_cmp crate for tests Nov 19, 2019
@rth rth merged commit afd7204 into rth:master Nov 19, 2019
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

2 participants