modified check_arrays to handle multiple sparse formats #3094

msalahi · 2014-04-21T05:02:57Z

on pull request #3076, it was proposed we extend the functionality of sklearn.utils.validation.check_arrays(...) to allow for multiple sparse formats. for example:

check_arrays(*arrays, sparse_format=['csc', 'csr'])

i did my best to make sure my implementation's compatible with both string arguements (sparse_format='csc') and collections (sparse_format=['csc', 'csr']). wasn't quite sure what to do in the case of something like...

check_arrays(*arrays, sparse_format=['csc', 'dense'])

...where both sparse and dense formats are listed. i assumed the most intuitive way to read that was to say "csc and dense arrays are allowed," and attempted to get the code to behave accordingly.

i could definitely use a watchful eye on this one, as check_arrays is used everywhere.

agramfort · 2014-04-21T06:54:26Z

sklearn/utils/validation.py

@@ -192,7 +192,8 @@ def check_arrays(*arrays, **options):
        Python lists or tuples occurring in arrays are converted to 1D numpy
        arrays, unless allow_lists is specified.

-    sparse_format : 'csr', 'csc' or 'dense', None by default
+    sparse_format : 'csr', 'csc', 'dense', or an iterable containing some


the type should fit on one line to render fine in the doc

I would write

sparse_format : 'csr' | 'csc' | 'dense' | list ... if is list ...

msalahi · 2014-04-22T01:39:00Z

addressed the issues @agramfort brought up: change documentation line, and avoid copying matrices to other types unnecessarily. There started to be way too much logic going on in check_arrays so i moved the sparse_format keyword arg validation and the array validation into two helper functions.

i decided that the most intuitive way to have the list argument to sparse_format work is to just have it accept ['csc' and/or 'csr']. 'dense,' and None can still be specified, they just can't be stuck in a list.

for example:

# OK
check_arrays(X, Y, sparse_format='csc')
check_arrays(X, Y, sparse_format=['csc'])
check_arrays(X, Y, sparse_format=['csc', 'csr']
check_arrays(X, Y, sparse_format='dense')

# not OK:

check_arrays(X, Y, sparse_format=['dense', 'csc'])
check_arrays(X, Y, sparse_format=[None]

finally, i added a little more test coverage around the sparse_format functionality

jnothman · 2014-04-22T12:09:20Z

sklearn/utils/validation.py

+        elif sparse_format is None:
+            return array
+        else:
+            input_format = array.__class__.__name__


Please use array.getformat()

Actually, there's array.format which is the same.

whoops, didn't realize that existed -- much nicer!

msalahi · 2014-04-23T15:24:09Z

8000

substituted my wonky array.__class__.__name__.split(' ')[0] for the much cleaner array.format to get a matrix's sparse format.

also added clearer documentation for the sparse_format keyword argument in check_arrays.

coveralls · 2014-04-23T15:32:17Z

Coverage remained the same when pulling a085b65 on msalahi:check-arrays-formats into feb6eb1 on scikit-learn:master.

ogrisel · 2014-05-12T13:47:42Z

sklearn/utils/tests/test_validation.py

@@ -102,6 +102,8 @@ def test_check_arrays_exceptions():
    assert_raises(TypeError, check_arrays, [0, 1], [0, 1], meaning_of_life=42)
    assert_raises(ValueError, check_arrays, [0], [0], sparse_format='fake')
    assert_raises(ValueError, check_arrays, np.zeros((2, 3, 4)), [0])
+    assert_raises(ValueError, check_arrays, [0], [0], sparse_format=['csc',
+                                                                     'fake'])


cosmetics, I would prefer:

assert_raises(ValueError, check_arrays, [0], [0], sparse_format=['csc', 'fake'])

amueller · 2015-01-16T21:03:57Z

I actually implemented this and also refactored all of the input validation in the last sprint. The check_arrays function is gone and we can allow multiple sparse formats. Therefore closing this.

amueller · 2015-01-16T21:05:14Z

For reference: #3443 and #3447

agramfort reviewed Apr 21, 2014
View reviewed changes

jnothman reviewed Apr 22, 2014
View reviewed changes

modified check_arrays to handle multiple sparse formats

a085b65

ogrisel reviewed May 12, 2014
View reviewed changes

larsmans force-pushed the master branch from 58a55ad to 4b82379 Compare August 25, 2014 21:50

MechCoder force-pushed the master branch from 6deaea0 to 3f49cee Compare November 3, 2014 12:36

amueller closed this Jan 16, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modified check_arrays to handle multiple sparse formats #3094

modified check_arrays to handle multiple sparse formats #3094

modified check_arrays to handle multiple sparse formats #3094

modified check_arrays to handle multiple sparse formats #3094

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment