[WIP] Fix read only mmap tests #5507
Conversation
pletelli
force-pushed
the
fix_read_only_mmap_tests
branch
from
47bea48
to
0d4ef7e
Compare
| @@ -381,7 +386,10 @@ def check_array(array, accept_sparse=None, dtype="numeric", order=None, | |||
| array = _ensure_sparse_format(array, accept_sparse, dtype, copy, | |||
| force_all_finite) | |||
| else: | |||
| array = np.array(array, dtype=dtype, order=order, copy=copy) | |||
| # Do not physically copy memory map : if type(array) == np.memmap: | |||
| # type(array) == np.array | |||
There was a problem hiding this comment.
The class is numpy.ndarray, never numpy.array.
|
Also see #4807 ? |
sklearn/utils/validation.py
Outdated
| @@ -341,9 +341,14 @@ def check_array(array, accept_sparse=None, dtype="numeric", order=None, | |||
| X_converted : object | |||
| The converted and validated X. | |||
| """ | |||
| if isinstance(array, np.memmap) and array.mode == "r" and not copy: | |||
| raise ValueError('Can not modify inplace an array which is read-only') | |||
|
|
|||
There was a problem hiding this comment.
this appears not to be the right place to raise that error.
check_array() can be called in both fit() and transform() as in RobustScaler for example.
The returned array is modified inplace in transform() but no modification is ever performed in fit().
Could we make this check at the beginning of each transform() method ? WDYT @ogrisel
This may be in the mother class's method fit_transform() and used calling super()
pletelli
force-pushed
the
fix_read_only_mmap_tests
branch
6 times, most recently
from
06ba2a8
to
5bfb9f7
Compare
pletelli
force-pushed
the
fix_read_only_mmap_tests
branch
from
5bfb9f7
to
d291d89
Compare
| X = check_array(X, dtype=np.float64, copy=self.copy) | ||
| Y = check_array(Y, dtype=np.float64, copy=self.copy, ensure_2d=False) | ||
| Xcopy = not X.flags.writable or self.copy | ||
| Ycopy = not Y.flags.writable or self.copy |
There was a problem hiding this comment.
this should maybe factored into check_array:
Parameters copy=False|True|'on_readonly'
|
@pletelli are you still willing to work on this or should I take up ? |
will soon fix #5481 which has been opened to fix tests on #4807
This PR is based on #4807