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

Require arrays to be equal…, & xfail equality of instances of stochastic classes #203

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JamesParrott
Copy link

not just either both all truthy or both not.

Fixes issue #202.

All the tests apart from the stochastic ones I've xfailed still pass - they were not testing much useful other than the correct
selection of MapClassifier sub classes by the factory function and assignemnt of args, but still:

Well done guys! This set of tests is much stricter, and they're all still passed. Arrays of random numbers should not be tested using equality, so I've just xfailed those tests until someone has a better suggestion.

@sjsrey
Copy link
Member

sjsrey commented May 29, 2024

not just either both all truthy or both not.

Fixes issue #202.

All the tests apart from the stochastic ones I've xfailed still pass - they were not testing much useful other than the correct selection of MapClassifier sub classes by the factory function and assignemnt of args, but still:

Well done guys! This set of tests is much stricter, and they're all still passed. Arrays of random numbers should not be tested using equality, so I've just xfailed those tests until someone has a better suggestion.

Shouldn't a set seed create the same sequence for the testing?

@JamesParrott
Copy link
Author

JamesParrott commented May 29, 2024

Sorry this was from 5 months ago, so let me summarise the PR. There are two main changes (that perhaps I should've split up):
i) replacing assert a.yb.all() == b.yb.all() etc. with assert (a.yb == b.yb).all() or similar,
and ii) xfailing the stochastic tests.

Presumably your comment below is in relation to ii)

Shouldn't a set seed create the same sequence for the testing?

"Should". The whole reason I xfailed the tests in the first place, was because I couldn't reproduce the passes.

The same set seed will produce the same sequences, only if the stochasticity in the test data and algorithms being tested is pseudo-random (i.e. not truly random), and only if everything else the pseudo-randomness sources use, are exactly the same in the environment used to produce the expected result, and in the test environment the tests are being run in since then.

The resulting tests will be brittle, flakey, and irreproducible in different test environments.

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