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
Remove all usages of numpy.compat ref #13880 #13889
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @harishsdev, thanks for the PR. This isn't quite right - I explained in more detail what to do.
Also, please write your code in PEP8 compatible fashion (e.g., spaces after commas).
scipy/_lib/_util.py
Outdated
def asbytes(s): | ||
if isinstance(s, bytes): | ||
return s | ||
return str(s).encode('latin1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are py2-py3 compatibility functions. Given that we only use Python 3, we should not need them. When removing them, the idea is to actually figure out if something is a string or bytes.
I'll give you one example for idl.py
.
scipy/io/idl.py
Outdated
@@ -31,7 +31,7 @@ | |||
|
|||
import struct | |||
import numpy as np | |||
from numpy.compat import asstr | |||
from scipy._lib._util import asstr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this import. Then look at where it is used. In this case in one place:
if length > 0:
chars = _read_bytes(f, length)
_align_32(f)
chars = asstr(chars)
else:
chars = ''
Now, you look at the file mode with which f
was opened. In this case that was f = open(file_name, 'rb')
. So f.read()
is guaranteed to give bytes
. Hence you can change the code to:
if length > 0:
chars = _read_bytes(f, length).decode('latin1')
_align_32(f)
else:
chars = ''
Then you run the tests to confirm - without .decode('latin1')
they all fail, with the decode added they all pass.
i tried to run tests using following command but i am facing issue, if its exists,please guide me how to proceed (pyart_env) PS D:\one\scipy> python .\runtests.py openblas_lapack_info: openblas_clapack_info: flame_info: atlas_3_10_threads_info: atlas_3_10_info: atlas_threads_info: atlas_info: lapack_info: lapack_src_info: NOT AVAILABLE Running from SciPy source directory. Build failed! (0:00:01.512716 elapsed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log tells you what's wrong at the bottom:
numpy.distutils.system_info.NotFoundError: No BLAS/LAPACK libraries found.
To build Scipy from sources, BLAS & LAPACK libraries need to be installed.
Perhaps this windows guide will help with installing libraries:
https://docs.scipy.org/doc/scipy/reference/building/windows.html#build-windows
scipy/io/matlab/mio5_utils.pyx
Outdated
@@ -918,7 +916,7 @@ cdef class VarReader5: | |||
char *n_ptr = names | |||
int j, dup_no | |||
for i in range(n_names): | |||
name = asstr(PyBytes_FromString(n_ptr)) | |||
name = PyBytes_FromString(n_ptr).decode('latin1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name = PyBytes_FromString(n_ptr).decode('latin1') | |
name = PyUnicode_FromString(n_ptr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is causing an error 'cpython/PyUnicode_FromString.pxd' not found
. I was also working on issue #13880, and I am new to cpython. How can I fix this?
scipy/io/matlab/mio5.py
Outdated
@@ -311,7 +309,7 @@ def get_variables(self, variable_names=None): | |||
mdict['__globals__'] = [] | |||
while not self.end_of_stream(): | |||
hdr, next_position = self.read_var_header() | |||
name = asstr(hdr.name) | |||
name = (hdr.name).decode('latin1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the unnecessary brackets here, and everywhere else.
name = (hdr.name).decode('latin1') | |
name = hdr.name.decode('latin1') |
stream.write((template % (real(aij), | ||
imag(aij).encode('latin1')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream.write((template % (real(aij), | |
imag(aij).encode('latin1')) | |
data = template % (real(aij), imag(aij)) | |
stream.write(data.encode('latin1')) |
stream.write((template % (real(aij), | ||
imag(aij))).encode('latin1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream.write((template % (real(aij), | |
imag(aij))).encode('latin1') | |
data = template % (real(aij), imag(aij)) | |
stream.write(data.encode('latin1')) |
scipy/io/mmio.py
Outdated
stream.write((("%i %i " % (r, c)) + | ||
(template % d)).encode('latin1')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream.write((("%i %i " % (r, c)) + | |
(template % d)).encode('latin1')) | |
stream.write((("%i %i " % (r, c)) + | |
(template % d)).encode('latin1')) |
scipy/io/mmio.py
Outdated
stream.write((("%i %i " % (r, c)) + | ||
(template % (d.real, d.imag))).encode('latin1')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream.write((("%i %i " % (r, c)) + | |
(template % (d.real, d.imag))).encode('latin1')) | |
stream.write((("%i %i " % (r, c)) + | |
(template % (d.real, d.imag))).encode('latin1')) |
Thanks for your contribution @harishsdev but the |
Reference issue
#13880
What does this implement/fix?
Remove all usages of numpy.compat(for asbytes and asstr functionality)
Additional information