Add warning for the scipy bug if numpy.float16 is passed. #15
Conversation
hetio/matrix.py
Outdated
| import numpy | ||
| import scipy.sparse | ||
|
|
||
| import hetio.hetnet |
There was a problem hiding this comment.
Do not move this import since it's a local import it goes in the third stanza.
hetio/matrix.py
Outdated
| warning(("scipy.sparse does not support the conversion " | ||
| "of numpy.float16 matrices to numpy.arrays. Converting " | ||
| "to numpy.float32 dtype")) | ||
| return matrix.astype(numpy.float32).toarray() |
There was a problem hiding this comment.
In this case, can we do:
matrix.astype(numpy.float32).toarray().astype(numpy.float16)
Let's also link to the issue in the warning.
hetio/matrix.py
Outdated
| return matrix.toarray() | ||
| except ValueError: | ||
| warning(("scipy.sparse does not support the conversion " | ||
| "of numpy.float16 matrices to numpy.arrays.")) |
There was a problem hiding this comment.
Can you add this link to this message: https://git.io/vpXyy
hetio/matrix.py
Outdated
| except ValueError: | ||
| warning(("scipy.sparse does not support the conversion " | ||
| "of numpy.float16 matrices to numpy.arrays.")) | ||
| return matrix.astype(numpy.float32).toarray().astype(numpy.float16) |
There was a problem hiding this comment.
Is there any way for us to pull the final dtype out of the input sparse matrix, so we guarantee that we convert it back to the same dtype dense matrix? Based on my reading of scipy/scipy#7408 (comment), I think it could be possible that other dtypes also trigger this error.
hetio/matrix.py
Outdated
| @@ -1,4 +1,5 @@ | |||
| from collections import OrderedDict | |||
| from logging import warning | |||
There was a problem hiding this comment.
I would prefer just doing import logging, especially since there is a builtin package called warnings.
Don't worry about exceed 80 characters per line. 100 characters is fine.
hetio/matrix.py
Outdated
| try: | ||
| return matrix.toarray() | ||
| except ValueError: | ||
| warning(("scipy.sparse does not support the conversion " |
There was a problem hiding this comment.
I don't think these inner parentheses are needed.
In reference to greenelab/connectivity-search-analyses#94 and scipy/scipy#7408. Rather than raising a
ValueErrorin a case like:it autoconverts to
numpy.float32and issues a warning.