-
Notifications
You must be signed in to change notification settings - Fork 228
[MRG] Quick fix of failed tests due to new scikit-learn version (0.20.0) #130
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
Merged
bellet
merged 2 commits into
scikit-learn-contrib:master
from
wdevazelhes:fix/fix_tests_sklearn_update
Oct 11, 2018
Merged
[MRG] Quick fix of failed tests due to new scikit-learn version (0.20.0) #130
bellet
merged 2 commits into
scikit-learn-contrib:master
from
wdevazelhes:fix/fix_tests_sklearn_update
Oct 11, 2018
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some iris tests are still failing ;-) |
Oh yes indeed, I used the wrong scikit-learn version for testing locally :p |
bellet
approved these changes
Oct 11, 2018
wdevazelhes
pushed a commit
to wdevazelhes/metric-learn
that referenced
this pull request
Oct 16, 2018
[MRG] Quick fix of failed tests due to new scikit-learn version (0.20.0) (scikit-learn-contrib#130) * TST: Quick fix of failed tests due to new scikit-learn version (0.20.0) * FIX update values to pass test
perimosocordiae
pushed a commit
that referenced
this pull request
Dec 14, 2018
* WIP create MahalanobisMixin * ENH Update algorithms with Mahalanobis Mixin: - Make them inherit from Mahalanobis Mixin, and implement the metric_ property - Improve metric_ property by checking if it exists and raising the appropriate warning if not - Make tests work, by replacing metric() with metric_ * FIX: add missing import * FIX: update sklearn's function check_no_fit_attributes_set_in_init to new check_no_attributes_set_in_init" This new function was introduced through PR scikit-learn/scikit-learn#9450 in scikit-learn. It also allows to pass tests that would otherwise not pass: indeed having abstract attributes as properties threw an error. But the new test functions handles well this property inheritance. * FIX: take function ``_get_args`` from scikit-learn's PR scikit-learn/scikit-learn#9450 Indeed, in the PR this function is modified to support python 2. This should solve the CI error. * ENH: add transformer_ attribute and improve docstring * WIP: move transform() in BaseMetricLearner to transformer_from_metric() in MahalanobisMixin * WIP: refactor metric to original formulation: a function, with result computed from the transformer * WIP: make all Mahalanobis Metric Learner algorithms have transformer_ and metric() * ENH Add score_pairs function - Make MahalanobisMixin inherit from BaseMetricLearner to give a concrete implementation of score_pairs - Use score_pairs to compute more easily predict - Add docstring - TST: for every algorithm: - test that using score_pairs pairwise returns an euclidean distance matrix - test that score_pairs works for 3D arrays of several pairs as well as 2D arrays of one pair (and there returns only a scalar) - test that score_pairs always returns a finite output * TST add test on toy example for score_pairs * ENH Add embed function - add the function and docstring - use it for score_pairs - TST : - should be finite - have right output dimension - embedding should be linear - should work on a toy example * FIX fix error in slicing of quadruplets * FIX minor corrections * FIX minor corrections - remove unusual s to test functions - remove redundant parenthesis * FIX fix PEP8 errors * FIX remove possible one-sample scoring from docstring for now * REF rename n_features_out to num_dims to be more coherent with current algorithms * MAINT: Adress #96 (review) - replace embed by transform and add always the input X in calling the function - mutualize _transformer_from_metric not to be overwritten in MMC - improve test_mahalanobis_mixin.test_score_pairs_pairwise according to #96 (comment) - improve test_mahalanobis_mixin.check_is_distance_matrix - correct typos and nitpicks * ENH: Add check_tuples * FIX: fix parenthesis * ENH: First commit adding a preprocessor * ENH: Improve check_tuples with more comments and deal better with ensure_min_features * STY: remove unexpected spaces * FIX: Raise more appropriate error message The previous error message would have said "[...], shape=(shape_of_the_2D_array_extracted_from_3D)" But it is clearer to print the shape of the actual 3D initial array (tuples in input) * FIX: fix string formatting and refactor name and context to use context in custom functions but to give name to check_array * FIX: only allow 2D if preprocessor and 3D if not preprocessor * FIX: put format arguments in the right order * MAINT: better to say the preprocessor than a preprocessor in messages * FIX: numeric should be default if NO preprocessor * FIX: preprocessor argument has to be a boolean (before this change was a callable or an array) * FIX: fix preprocessor argument passed in check_tuples in base_metric * MAINT: say a preprocessor rather than the preprocessor * DOC: fix docstring of t in check_tuples * MAINT: make error messages better by only printing presence of preprocessor if the error is because not 2D or 3D shape * TST: Add tests for check_tuples * TST: simplify tests by removing the test for messages with the estimator name, but incorporating it in all other tests through parametrization * STY: remove unnecessary parenthesis * FIX: put back else statement that probably was wrongfully merged * TST: add tests for weakly supervised estimators and preprocessor that fetches indices * TST: add tests for preprocessor * FIX: remove redundant metric and transformer function, wrongly merged * MAINT: rename format_input into preprocess_tuples and input into tuples * MAINT: fixes and enhancements - fix the format_input previous incomplete refactoring - mututalize check_tuples for Weakly Supervised Algorithms - fix test of string representations * MAINT: mutualize check_tuples * MAINT: refactor SimplePreprocessor into ArrayIndexer * MAINT: improve check_tuples and tests * TST: add random seed for _Supervised classes * TST: Adapt test pipeline - use random state for _Supervised classes - call transform only for pipeline with a TransformerMixin * TST: fix test_progress_message_preprocessor_tuples by making func return an np.array * Remove deprecated cross_validation import and put model_selection instead * WIP replace checks by unique check_input function * Fixes some tests: - fixes dtype checks and conversion by ensuring the checks and conversions are made on the preprocessed array and not the input array (which can be an array of indices) - fix tests that were failing due to the wrong error message * TST: Cherry pick from new sklearn version ac0e230 [MRG] Quick fix of failed tests due to new scikit-learn version (0.20.0) (#130) * TST: Quick fix of failed tests due to new scikit-learn version (0.20.0) * FIX update values to pass test * FIX: get changes from master to pass test iris for NCA * FIX fix tests that were failing due to the error message FIX check_tuples at the end and only at the end, because the number of tuples should not be modified from the beginning, and we want to check them also at the end * TST: fix test_check_input_invalid_t that changed since we test t at the end now * TST fix NCA's iris test taking code from master * FIX fix tests: - use check_array instead of conversion to numpy array to ensure that sparse array are kept as such and not wrapped into a regular numpy array - check_array the metric with the right arguments for covariance in order not to fail if the array is a scalar (for one-feature samples) * FIX fix previous modification that removed self.X_ but was modifying the input at fit time * FIX ensure at least 2d only for checking the metric because after check_array of inputs the metric should be a scalar or an array so we only need to ensure it is an array (2D array) * STY: Fix PEP8 violations * MAINT: Refactor error messages with the help of numerical codes * MAINT: mutualize check_preprocessor and check_input for every estimator * FIX: remove format_map for python2.7 compatibility * DOC: Add docstring for check_input and fix some bugs * DOC: add docstrings * MAINT: Removing changes not related to this PR, and fixing previous probable unsuccessfull merge * STY: Fix PEP8 errors * STY: fix indent problems * Fixing docstring spaces * DOC: add preprocessor docstring when missing * STY: PEP8 fixes * MAINT: refactor the global check function into _prepare_input * FIX: fix quadruplets scoring and delete useless comments * MAINT: remove some enhancements to be coherent with previous code and simplify review * MAINT: Improve test messages * MAINT: reorganize tests * FIX: fix typo in LMNN shogun and clean todo for the equivalent code in python_LMNN * MAINT: Rename inputs and input into input_data * STY: add backticks to None * MAINT: add more detailed comment of first checks and remove old comment * MAINT: improve comments for checking num_features * MAINT: Refactor t into tuple_size * MAINT: Fix small PEP8 error * MAINT: FIX remaining t into tuple_size and replace hasattr if None by getattr with default None * MAINT: remove misplaced comment * MAINT: Put back/add docstrings for decision_function/predict * MAINT: remove unnecessary ellipsis and upadate docstring of decision_function * Add comments in LMNN for arguments useful for the shogun version that are not used in the python version * MAINT: Remove useless mock_preprocessor * MAINT: Remove useless loop * MAINT: refactor test_dict_unchanged * MAINT: remove _get_args copied from scikit-learn and replace it by an import * MAINT: Fragment check_input by extracting blocks into check_input_classic and check_input_tuples * MAINT: ensure min_samples=2 for supervised learning algorithms (we should at least have two datapoints to learn something) * ENH: Return custom error when some error is due to the preprocessor * MAINT: Refactor algorithms preprocessing steps - extract preprocessing steps in main fit function - remove self.X_ when found and replace it by X (Fixes #134) - extract the function to test collapsed pairs as _utils.check_collapsed_pairs and test it - note that the function is now not used where it was used before but the user should be responsible for having coherent input (if he wants he can use the helper function as a preprocessing step) * MAINT: finish the work of the previous commit * TST: add test for cross-validation: comparison of manual cross-val and scikit-learn cross-val * ENH: put y=None by default in LSML for better compatibility. This also allows to simplify some tests by removing the need for a separate case for lsml * ENH: add error message when type of inputs is not some expected type * TST: add test that checks that 'classic' is the default behaviour * TST: remove unnecessary conversion to vertical vector of y * FIX: remove wrong condition hasattr 'score' at top of loop * MAINT: Add comment to explain why we return twice X for build_regression and build_classification * ENH: improve test for preprocessor and return error message if the given preprocessor has the bad type * FIX: fix wrong type_of_inputs in a test * FIX: deal with the case where preprocessor is None * WIP refactor build_dataset * MAINT: refactor bool preprocessor to with_preprocessor * FIX: fix build_pairs and build_quadruplets because 'only named arguments should follow expression' * STY: fix PEP8 error * MAINT: mututalize test_same_with_or_without_preprocessor_tuples and test_same_with_or_without_preprocessor_classic * TST: give better names in test_same_with_or_without_preprocessor * MAINT: refactor list_estimators into metric_learners * TST: uniformize names input_data - tuples, labels - y * FIX: fix build_pairs and build_quadruplets * MAINT: remove forgotten code duplication * MAINT: address #117 (review)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #126 #127
This is a quick fix that fixes #126 and #127, by setting
ensure_min_samples=2
in checks of the inputs to pass scikit-learn's check_estimator test (see #127), and by updating the scores with the new values for the iris dataset.But I opened two issues that tackle solving these problems in the long term (cf #127)