Removed yield tests which are depricated in py.test#7654
Removed yield tests which are depricated in py.test#7654Erotemic wants to merge 1 commit intoscikit-learn:masterfrom
Conversation
|
After looking into this a bit more I realized you guys primarily use nose/unittest, not py.test. It seems like py.test just happens to mostly work with the testing framework as well. Please disregard this initial commit. However, while inspecting the tests I have noticed something that is potentially an issue. If you run The decorator seems to be behaving oddly with the yield tests. I'm currently looking into a potential solution which may continue to support py.test as well as nose/unittest. |
…d old yield tests that were not running due to wrapping issue with if_32bit.
|
I think we need to officially adopt pytest for this to be appropriate. Currently the yield tests are useful with nosetest. |
fb94f92 to
41d023b
Compare
|
I've gone through with a first pass at a decorator that can support both frameworks. @jnothman Fair point, its likely not worth the effort to maintain compatibility. However, there still is the issue of the sklearn.ensemble.tests.test_forest:test_importances test which was never actually running. I think I've fixed the issue in this branch, but I'll close the issue and open a new smaller PR that specifically fixes that issue. |
|
There seems to be interest in moving to py.test, in which case this PR, or On 13 October 2016 at 09:42, Jon Crall notifications@github.com wrote:
|
|
Ah, in that case I'll keep this closed but I won't delete the branch. I assume things will change by the time the library moves to py.test, so I'll share the script I used to make the more tedious changes (decorating all of the yield tests to work with py.test). The Here is the script I used. It requires my utility library as well as the redbaron (a full syntax tree parsing library) to run in addition to the obvious changes of the paths. # command to find the files that probably need changing.
# grep -R --include test_*.py "^ *yield" .
# grep -lR --include test_*.py "^ *yield" . | xargs readlink -f
fpath_list = '''
/home/joncrall/code/scikit-learn/sklearn/tree/tests/test_tree.py
/home/joncrall/code/scikit-learn/sklearn/preprocessing/tests/test_label.py
home/joncrall/code/scikit-learn/sklearn/tests/test_naive_bayes.py
/home/joncrall/code/scikit-learn/sklearn/tests/test_common.py
/home/joncrall/code/scikit-learn/sklearn/tests/test_cross_validation.py
/home/joncrall/code/scikit-learn/sklearn/tests/test_random_projection.py
/home/joncrall/code/scikit-learn/sklearn/mixture/tests/test_gmm.py
/home/joncrall/code/scikit-learn/sklearn/ensemble/tests/test_forest.py
/home/joncrall/code/scikit-learn/sklearn/ensemble/tests/test_gradient_boosting.py
/home/joncrall/code/scikit-learn/sklearn/neighbors/tests/test_ball_tree.py
/home/joncrall/code/scikit-learn/sklearn/neighbors/tests/test_neighbors.py
/home/joncrall/code/scikit-learn/sklearn/neighbors/tests/test_dist_metrics.py
/home/joncrall/code/scikit-learn/sklearn/neighbors/tests/test_kd_tree.py
/home/joncrall/code/scikit-learn/sklearn/neighbors/tests/test_kde.py
/home/joncrall/code/scikit-learn/sklearn/utils/tests/test_stats.py
/home/joncrall/code/scikit-learn/sklearn/gaussian_process/tests/test_kernels.py
/home/joncrall/code/scikit-learn/sklearn/metrics/tests/test_common.py
/home/joncrall/code/scikit-learn/sklearn/metrics/tests/test_pairwise.py
/home/joncrall/code/scikit-learn/sklearn/metrics/tests/test_ranking.py
/home/joncrall/code/scikit-learn/sklearn/metrics/tests/test_score_objects.py
/home/joncrall/code/scikit-learn/sklearn/svm/tests/test_bounds.py
/home/joncrall/code/scikit-learn/sklearn/linear_model/tests/test_theil_sen.py
/home/joncrall/code/scikit-learn/sklearn/linear_model/tests/test_ridge.py
/home/joncrall/code/scikit-learn/sklearn/model_selection/tests/test_validation.py
'''.strip('\n').split('\n')
# fpath = fpath_list[0]
# import utool as ut
# ut.rrrr(0)
# self = ut.BaronWraper.from_fpath(fpath)
# My utility library
# pip install utool
# requires redbaron
# pip install redbaron
import utool as ut
for fpath in ut.ProgIter(fpath_list[1:], 'fpaths'):
self = ut.BaronWraper.from_fpath(fpath)
def is_decorated_with(func_node, target_name):
# Find funcs decorated with does yield
return any(d.value.dumps() == target_name
for d in func_node.decorators)
# Find all relevant functions that need decoration
target_name = 'does_yield'
yields = self.baron.find_all('yield')
needs_fix = set([])
for yield_node in ut.ProgIter(yields, 'finding yields'):
func_node = self.find_root_function(yield_node)
if not is_decorated_with(func_node, target_name):
needs_fix.add(func_node)
# Decorate the found functions
for func_node in ut.ProgIter(needs_fix, 'fixing decors'):
func_node.decorators.insert(0, '@' + target_name)
# Ensure function is properly imported
if not self.baron.find_all('nameasname', target_name):
fromimport = self.baron.find_all('FromImport')
testing_fromimports = [node for node in fromimport if 'sklearn.utils.testing' in node.dumps()]
last_fromimport = testing_fromimports[-1]
last_fromimport.insert_after('from sklearn.utils.testing import ' + target_name)
# Write the new file to disk
self.write() |
|
we can use parametrized tests instead of the yields.. the yields are all in one place (or two, there's another one in the metrics) |
|
oh wow there is way more yields than I thought. never mind. But they can all be replaced by parametrized tests if we move to py.test. |
Running tests on my machine results in a large amount of warnings from py.test about yield tests.
I'm running pytest version 3.0.1. An example of a warning line is:
The warning repeats for over 200 instances of this problem.
Each instance of the yield tests looks like it was generating a tuple of a function and its arguments.
To resolve the issue I simply called the function instead of yielding the tuple.