bootphon / pygamma-agreement

Gamma Agreement in Python
MIT License
40 stars 8 forks source link

Warning from deprecated use of `numpy` builtin types. #20

Closed apiad closed 3 years ago

apiad commented 3 years ago

Greetings. I'm one of the reviewers assigned to the submission to JOSS (https://github.com/openjournals/joss-reviews/issues/2989).

I'm in the process of downloading the package and running the tests. All tests worked correctly, however, there are several deprecation warnings from numpy.

I'm opening this issue basically to understand what could be the impact of these warnings, if they are easy to fix, or if they are irrelevant.

hadware commented 3 years ago

hey @apiad , thanks again for reviewing this package.

I'll update my local numpy version to see if I get the same warnings. In the meantime, can you specify your numpy version as to make sure that we are on the same page?

apiad commented 3 years ago

Sure thing! I just pip installed on a fresh environment. It installed numpy==1.20.1.

apiad commented 3 years ago

This is one of the messages:

  /home/apiad/.local/lib/python3.8/site-packages/cvxpy/interface/numpy_interface/ndarray_interface.py:47: DeprecationWarning: `np.complex` is a deprecated alias for the builtin `complex`. To silence this warning, use `complex` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.complex128` here.
  Deprecated in NumPy 1.20; for more details and guidance: https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations
    if result.dtype in [numpy.complex, numpy.float64]:

Again, I don't really think this is extremely important, not at all, but since we are supposed to verify the functionality claims, just wanted to double check if you are OK with these warnings.

hadware commented 3 years ago

Alright, I did my updates, and they're all coming from numpy usages from numba, cvxpy and ecos (dependency of cvxpy). It's ugly, but there's not much i can do :man_shrugging: .

Do you think i should silence warnings (like this https://docs.python.org/3/library/warnings.html#temporarily-suppressing-warnings ) ?

apiad commented 3 years ago

I'm not sure. If those warnings will appear to the users of your API and you are positive you understand why they are happening, I think it might be a good idea to silence it, so as to not disturb the user with "issues" that they do not understand and can't do anything about. But silencing warnings is always complicated, IMHO, because something important might slip under the radar.

Anyway, on my side, I think I'm pretty much convinced these warnings are not relevant to the review process as they do not hinder in any way the functionalities of the library, so I'll be closing this issue.

Thanks!