search

alan-turing-institute / deepsensor

A Python package for tackling diverse environmental prediction tasks with NPs.
https://alan-turing-institute.github.io/deepsensor/
MIT License
72 stars 15 forks source link

use unittest's `setUpClass` instead of overriding `__init__` #117

Closed davidwilby closed 2 months ago

davidwilby commented 3 months ago

Hopefully this resolves #115 :crossed_fingers:

Whilst investigating the issue raised in #115 I noticed a number of issues on which the order/combination of tests run influenced the behaviour.

I can't say that I fully follow the reason behind the pollution between the different test modules but these changes seem to produce the desired behaviour as far as I can tell :shrug:

UnitTest recommends using the class method setUpClass (similar to setUp but runs only once for all of the tests in the class)

Have run this locally and all tests pass, let's see what happens on GitHub actions...

Note that this branch doesn't include the fixes to the CI testing workflow in #116 so technically the tests on this branch will be running with the runner's OS python installation, not those managed by the python version matrix.

tom-andersson commented 2 months ago

Tyvm @davidwilby! CI looks happy, so let's go ahead and merge and (fingers crossed) once tests are passing on main we can assume this resolves #115.

Note: Reminder to run black just before committing (I've done it for you in this instance)

tom-andersson commented 2 months ago

Yup, tests were failing just before merging this PR, and are now passing. Nice!

Perhaps import deepsensor.torch in test_model wasn't playing well with import deepsensor.tensorflow in test_active_learning, triggering backends to complain about mixed types. I don't fully understand why this test leakage would start becoming a problem seemingly "out of nowhere" though. Oh well, it's fixed now.

tom-andersson commented 2 months ago

@all-contributors please add @davidwilby for test

allcontributors[bot] commented 2 months ago

@tom-andersson

I've put up a pull request to add @davidwilby! :tada: