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

Remove deprecated np.int and np.float dependencies #797

Merged
merged 5 commits into from Oct 6, 2022
Merged

Conversation

geordie666
Copy link
Contributor

This PR addresses #790 (as well as fixing another couple of minor issues).

Sorry this took so long, I was expecting to need to update many occurrences of np.int and np.float and hadn't realized there were only a few.

TMI: The code I ran to check my casting choices were correct for the specific examples mentioned in #790 was:

Grr1 = 45.6
Grr2 = np.array([45.6])
Grr3 = np.array([45.6, 65.7])

for Grr in [Grr1, Grr2, Grr3]:
    print(Grr, isinstance(Grr, np.float))

<ipython-input-12-f43a48c0ecf8>:2: DeprecationWarning: `np.float` is a deprecated alias for the builtin `float`. To sile
nce this warning, use `float` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted
 the numpy scalar type, use `np.float64` here.
Deprecated in NumPy 1.20; for more details and guidance: https://numpy.org/devdocs/release/1.20.0-notes.html#deprecation
s
  print(Grr, isinstance(Grr, np.float))

45.6 True
[45.6] False
[45.6 65.7] False

for Grr in [Grr1, Grr2, Grr3]:
    print(Grr, isinstance(Grr, float))

45.6 True
[45.6] False
[45.6 65.7] False

for Grr in [Grr1, Grr2, Grr3]:
    print(Grr, isinstance(Grr, np.float64))

45.6 False
[45.6] False
[45.6 65.7] False

and

nside_chunk1, nside1 = np.array([32, 32, 64]), np.array([16, 8, 16])
nside_chunk2, nside2 = np.array([32]), np.array([16])
nside_chunk3, nside3 = 32, 16
for nside_chunk, nside in zip([nside_chunk1, nside_chunk2, nside_chunk3], [nside1, nside2, nside3]):
    try:
        nchunk = 4**np.int(np.log2(nside_chunk) - np.log2(nside))
        print(nside_chunk, nchunk)
    except TypeError:
        print("Casting {} and {} wasn't possible anyway".format(nside_chunk, nside))

<ipython-input-4-9ca0a781b651>:6: DeprecationWarning: `np.int` is a deprecated alias for the builtin `int`. To silence this warning, use `int` by itself. Doing this will not modify any behavior and is safe. When replacing `np.int`, you may wish to use e.g. `np.int64` or `np.int32` to specify the precision. If you wish to review your current use, check the release note link for additional information.
Deprecated in NumPy 1.20; for more details and guidance: https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations
  nchunk = 4**np.int(np.log2(nside_chunk) - np.log2(nside))
Casting [32 32 64] and [16  8 16] wasn't possible anyway
[32] 4
32 4

nside_chunk1, nside1 = np.array([32, 32, 64]), np.array([16, 8, 16])
nside_chunk2, nside2 = np.array([32]), np.array([16])
nside_chunk3, nside3 = 32, 16
for nside_chunk, nside in zip([nside_chunk1, nside_chunk2, nside_chunk3], [nside1, nside2, nside3]):
    try:
        nchunk = 4**int(np.log2(nside_chunk) - np.log2(nside))
        print(nside_chunk, nchunk)
    except TypeError:
        print("Casting {} and {} wasn't possible anyway".format(nside_chunk, nside))

Casting [32 32 64] and [16  8 16] wasn't possible anyway
[32] 4
32 4

@coveralls
Copy link

coveralls commented Oct 5, 2022

Coverage Status

Coverage decreased (-11.5%) to 44.728% when pulling 059fa96 on ADM-dep-fixes into 692bd31 on main.

@geordie666
Copy link
Contributor Author

Before we merge this I should try to work out why coverage has dropped so much. I'm not sure how I could have reduced coverage by 11% with only a dozen or so changes, but I'll investigate.

@geordie666
Copy link
Contributor Author

I'm confused about the drop in coverage. Running pytest --cov on Cori for this branch and for main create coverage reports that differ by only three lines, corresponding to the three lines of code I removed from randoms.py. So, at NERSC I obtain coverage results that are completely aligned with my expectation.

I'm therefore going to assume that the drop in coverage from 56.222% to 44.728% was a glitch in coveralls.

@sbailey: I think I'm ready to merge this branch, unless you have further feedback regarding my comments about why nside_chunk can't be array-like.

@weaverba137
Copy link
Member

@geordie666, I would not get into the habit of blaming coveralls on this. I have found them to be highly reliable. The more likely explanation is that tests are being skipped in the GitHub Actions environment.

@sbailey sbailey merged commit c5e89fa into main Oct 6, 2022
@sbailey sbailey deleted the ADM-dep-fixes branch October 6, 2022 18:31
@geordie666
Copy link
Contributor Author

@weaverba137: Sure, agreed. I used sloppy language. I should have stated "a glitch or change in something leading up to and including the coveralls step."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants