scot-dev / scot

EEG/MEG Source Connectivity Toolbox in Python
http://scot-dev.github.io/scot-doc/index.html
MIT License
58 stars 23 forks source link

Add testing env without optional dependencies #170

Closed cbrnr closed 9 months ago

cbrnr commented 8 years ago

Fixes #166

cbrnr commented 8 years ago

Seems like the envs are working - but of course the first test in the minimal env has already failed because there is no sklearn :-).

cbrnr commented 8 years ago

Actually, only two tests seem to be affected: test_var_sklearn.py and test_xvschema.py. How do we proceed @mbillingr? Decorate all tests that require sklearn?

mbillingr commented 8 years ago

Do we have to decorate each function separately or can we decorate the whole class?

cbrnr commented 8 years ago

No idea - I guess decorators only work with functions?

mbillingr commented 8 years ago

You can also decorate classes (at least in Python 3, don't know about 2.7).

cbrnr commented 8 years ago

This would be nice, because in principle we could skip the whole Python file in some cases...

Can you check if the decorators from my last commit could work? How would you apply them to classes?

cbrnr commented 8 years ago

Before this whole decorator thing gets out of control - since we wrap all our tests in classes derived from unittest.TestCase anyway, can't we just require the import in the setUp method? Then maybe there's no need to create decorators at all.

cbrnr commented 8 years ago

There is @unittest.skipIf - maybe we can use this decorator for our needs?

mbillingr commented 8 years ago

Before this whole decorator thing gets out of control - since we wrap all our tests in classes derived from unittest.TestCase anyway, can't we just require the import in the setUp method? Then maybe there's no need to create decorators at all.

Which means we have to repeat these boilerplate checks. If not too many tests are affected this would be OK.

There is @unittest.skipIf - maybe we can use this decorator for our needs?

Maybe. We'd still have to pass a condition.

cbrnr commented 8 years ago

The solution in my last commit works. However, the following tests fail because they rely on all backends:

And probably even more.

This shows that our optional dependencies are heavily worked into the tests. It will probably be quite painful to tease these tests apart. The alternative is to make sklearn and matplotlib required deps.

mbillingr commented 8 years ago

I would rather solve this problem by changing the backends so that they do not register themselves with the manager if their requirements are not met.

e.g. backend_sklearn.py (line 96+)

try:
    import sklearn
    backend.register('sklearn', generate)
except ImportError:
    pass
cbrnr commented 8 years ago

Very good - even cleaner with an else branch. What else?

mbillingr commented 8 years ago

Nice! I did't know you coud use else with try/except.

cbrnr commented 8 years ago

I wonder

  1. why the non-MKL env doesn't work (again!)
  2. where the error in the 3.5 MKL env occurs (the log isn't very specific)
mbillingr commented 8 years ago
  1. It's this awful libgfortran again :angry:
  2. The log can't be more specific because the error happens in run_tests.sh line 14. These lines should take the possibility into account that the libraries are not available...
cbrnr commented 8 years ago

Re 2, I've fixed the broken run_tests.sh script. What do we do about 1? It seems like Anaconda is not really interested in keeping specific old package versions around - in our 2.7 env, it automatically updates

I think the best thing to do would be to just use whatever package versions the current Anaconda distribution ships (and completely ignore non-MKL because the Anaconda default is MKL). To test the oldest supported packages, we should create a non-Anaconda-based Python env (e.g. installed via Ubuntu packages).

I.e., I suggest the following testing envs:

cbrnr commented 8 years ago
cbrnr commented 8 years ago

@mbillingr do you know why the tests now fail after I've moved the imports into setUp?

mbillingr commented 8 years ago

do you know why the tests now fail after I've moved the imports into setUp?

Because they are imported locally. Well, OK.. they are just imported but the variables that refer to the modules are local to the setUp function. For solutions see StackOverflow.

cbrnr commented 8 years ago

Hm. I thought they were known to the object. What do you think is the best solution here? Make plt global?

In general, I have the feeling that mixing core tests with optional tests is rather ugly. Can't we do something nicer, such as completely separate core tests from optional tests?

mbillingr commented 8 years ago

What do you think is the best solution here? Make plt global?

Global would work, but that is kinda ugly. self.plt could work too, but may require many changes. It's also possible to catch import errors and skip certain tests if plt does not exist.

Can't we do something nicer, such as completely separate core tests from optional tests?

Sure. We even have the option to separate them at different levels:

  1. Some of the test cases are optional methods and can be skipped (I guess that's the current approach)
  2. Make another class for optional tests
  3. Make separate files for optional tests
cbrnr commented 8 years ago

Hm. With option 2, we wouldn't have to change the file structure, and we could try/except import errors to see if the tests in the optional class should be run. Shall we go for it?

mbillingr commented 8 years ago

Hm. With option 2, we wouldn't have to change the file structure, and we could try/except import errors to see if the tests in the optional class should be run. Shall we go for it?

:+1:

cbrnr commented 9 months ago

Closing, if we ever need this, it is probably easier to start from scratch.