Skip to content
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

Fix a numpy deprecation related to dtype #1149

Closed
wants to merge 1 commit into from

Conversation

Armavica
Copy link

Deprecated in numpy 1.21.0.
This fixes 369 deprecation warnings in PyMC's test suite.

See https://numpy.org/doc/stable/release/1.21.0-notes.html#the-dtype-attribute-must-return-a-dtype

Thank you for opening a PR!

Here are a few important guidelines and requirements to check before your PR can be merged:

  • There is an informative high-level description of the changes.
  • The description and/or commit message(s) references the relevant GitHub issue(s).
  • pre-commit is installed and set up.
  • The commit messages follow these guidelines.
  • The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR.
  • There are tests covering the changes introduced in the PR.

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

Deprecated in numpy 1.21.0.
This fixes 369 deprecation warnings in PyMC's test suite.

See https://numpy.org/doc/stable/release/1.21.0-notes.html#the-dtype-attribute-must-return-a-dtype
Copy link
Member

@brandonwillard brandonwillard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I just fixed a related issue in 8867a72. Can you try the same numpy.obj2sctype approach from there? It appears to be a concise way to determine whether or not a string or type is a valid dtype.

@brandonwillard brandonwillard added bug Something isn't working refactor This issue involves refactoring NumPy compatibility labels Aug 26, 2022
@Armavica
Copy link
Author

Armavica commented Aug 26, 2022

I am sorry, but I am not sure how this would work. The function obj2sctype doesn't seem to return either a dtype or an object whose .dtype is a dtype, as shown by

for a in ["float64", np.int8, np.int8(4).dtype]:
    assert not isinstance(np.obj2sctype(a), np.dtype)
    assert not isinstance(np.obj2sctype(a).dtype, np.dtype)

It is possible that I am missing something though, as this dtype manipulation is extremely confusing to me.

@brandonwillard
Copy link
Member

I am sorry, but I am not sure how this would work. The function obj2sctype doesn't seem to return either a dtype or an object whose .dtype is a dtype, as shown by

for a in ["float64", np.int8, np.int8(4).dtype]:
    assert not isinstance(np.obj2sctype(a), np.dtype)
    assert not isinstance(np.obj2sctype(a).dtype, np.dtype)

It is possible that I am missing something though, as this dtype manipulation is extremely confusing to me.

Likewise, I'm probably not thinking of the same issue that you are.

Are the deprecation warnings caused by something like the following?

import numpy as np
import aesara.tensor as at


np.dtype(at.vector("x"))
# <ipython-input-11-4480ef785bc5>:1: DeprecationWarning: in the future the `.dtype` attribute of a given datatype object must be a valid dtype instance. `data_type.dtype` may need to be coerced using `np.dtype(data_type.dtype)`. (Deprecated NumPy 1.20)
#   np.dtype(at.vector("x"))

@Armavica
Copy link
Author

Yes, I think that is the idea, at least for some of the warnings.

@brandonwillard
Copy link
Member

Yes, I think that is the idea, at least for some of the warnings.

I think I found a test that provides an example of the warnings you're talking about: pymc/tests/test_logprob.py::test_joint_logp_basic.

In that case, pymc.distributions.logprob.joint_logp is passing a TensorType—via var[0].type—to the TensorVariable.astype method here, when it should be using var[0].dtype.

@Armavica
Copy link
Author

Amazing, it looks like this fixes everything! Thank you very much!

@Armavica Armavica closed this Aug 27, 2022
Armavica added a commit to Armavica/pymc that referenced this pull request Aug 27, 2022
ricardoV94 pushed a commit to pymc-devs/pymc that referenced this pull request Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working NumPy compatibility refactor This issue involves refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants