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

Reduce DeprecationWarnings in Testing #582

Merged
merged 23 commits into from
Apr 22, 2024
Merged

Conversation

cyschneck
Copy link
Contributor

@cyschneck cyschneck commented Mar 22, 2024

PR Summary

Closes #477

PR Checklist

General

  • Make an issue if one doesn't already exist
  • Link the issue this PR resolves by adding closes #XXX to the PR description where XXX is the number of the issue.
  • Add a brief summary of changes to docs/release-notes.rst in a relevant section for the next unreleased release. Possible sections include: Documentation, New Features, Bug Fixes, Internal Changes, Breaking Changes/Deprecated
  • Add appropriate labels to this PR
  • Make your changes in a forked repository rather than directly in this repo
  • Open this PR as a draft if it is not ready for review
  • Convert this PR from a draft to a full PR before requesting reviewers
  • Passes precommit. To set up on your local, run pre-commit install from the top level of the repository. To manually run pre-commits, use pre-commit run --all-files and re-add any changed files before committing again and pushing.
  • If needed, squash and merge PR commits into a single commit to clean up commit history

@cyschneck cyschneck added the testing Issue related to testing label Mar 22, 2024
@cyschneck cyschneck self-assigned this Mar 22, 2024
@cyschneck
Copy link
Contributor Author

The filter in test_dask is for the warning RuntimeWarning: invalid value encountered in sqrt and filtering it is based on how xarray handles the same issue that dask is triggering

The filters in test_meteorology are warnings for when heatindex is attempting to take the square root of a negative number. Numpy doesn't fail when attempting a square root, but converts the negative number into a nan

import numpy as np
np.sqrt(np.array([-1, 0, 1]))

<stdin>:1: RuntimeWarning: invalid value encountered in sqrt
array([nan,  0.,  1.])

This PR is going to end up leaving two deprecation warnings from test_stats.py. These warnings are for eofuc and eofunc_ts. These are both warnings that originate in geocat-comp stats.py that are used for backwards compatibility.

The remaining geocat warning is for interpolation.py:133 where UserWarning: Interpolation point out of data bounds encountered return func_interpolate(new_levels, xcoords, data, axis=interp_axis) when the func_interpolate is using metpy.interpolate.interpolate_1d function

@cyschneck cyschneck marked this pull request as ready for review March 26, 2024 18:44
Copy link
Contributor

@kafitzgerald kafitzgerald left a comment

Choose a reason for hiding this comment

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

I'm a little concerned some of these might also be worth looking into a little more deeply and potentially addressing via better handling or tests in geocat-comp.

test/test_interpolation.py Outdated Show resolved Hide resolved
test/test_meteorology.py Outdated Show resolved Hide resolved
@cyschneck
Copy link
Contributor Author

cyschneck commented Apr 4, 2024

Sounds good, the interpolate function is a lot of interconnected pieces. Let me write up more about how it is populated

@cyschneck
Copy link
Contributor Author

The interpolation warning is generated by the test test_interp_hybrid_to_pressure_extrap_geopotential that is populated from press_in which is populated with the values

[[[68619.195 68619.195]
  [69344.43  69290.68 ]
  [70799.516 70643.86 ]]]

To be interpolated along the values

[ 50000  92500  95000 100000]

The test calls interp_hybrid_to_pressure as linear, so it ends up calling metpy interpolate_1d. Specifically, the error being thrown is because the interpolate values and the coordinates shapes are equal without a fill_value

Metpy will raise a warning that:

Warn if interpolated values are outside data bounds, will make these the values at end of data range.

Since the input values max value 70272.49076537043 is less than max interpolate value 100000 it can't interpolate to the bounds outside the range (without setting a fill_value). So, by default, It adds the values to the end of the range

An easy way to see this is interpolating to 4.5 when the range is to 4:

import metpy.interpolate
import numpy as np

x = np.array([1., 2., 3., 4.])
y = np.array([1., 2., 3., 4.])
x_interp = np.array([2.5, 3.5])
metpy.interpolate.interpolate_1d(x_interp, x, y)

Where the array position of the max value is out of bounds, so the array appends nan

metpy.interpolate.interpolate_1d(x_interp, x, y)
<stdin>:1: UserWarning: Interpolation point out of data bounds encountered
array([2.5, nan])

When the values are within range, it returns the interpolated values without a nan

import metpy.interpolate
import numpy as np

x = np.array([1., 2., 3., 4.])
y = np.array([1., 2., 3., 4.])
x_interp = np.array([2.5, 3.5])
metpy.interpolate.interpolate_1d(x_interp, x, y)
array([2.5, 3.5])

@anissa111
Copy link
Member

I think I may be a little confused about what's intended to be in this PR. Is this PR supposed to specifically address deprecation warnings only? Also, since this PR will close #477, I'd like to see more of the warnings discussed in that issue addressed here.

docs/release-notes.rst Outdated Show resolved Hide resolved
test/test_interpolation.py Outdated Show resolved Hide resolved
@cyschneck
Copy link
Contributor Author

Added new issue to address UserWarning in interpolation.py (#593)

@cyschneck cyschneck added the documentation Improvements or additions to documentation label Apr 10, 2024
docs/contrib.rst Outdated Show resolved Hide resolved
.github/workflows/pypi.yaml Show resolved Hide resolved
docs/release-notes.rst Show resolved Hide resolved
geocat/comp/interpolation.py Outdated Show resolved Hide resolved
test/test_dask.py Outdated Show resolved Hide resolved
@cyschneck
Copy link
Contributor Author

Updated scripts to account for deprecation warnings for using setup.py as well as warnings associated with redirecting links in the docs
Test scripts will continue to throw warnings associated with dask data.chunks, heat_index, and interpolation out of bounds, since these will be handled in upcoming issues (#593 and #596)

@anissa111 anissa111 self-requested a review April 22, 2024 16:09
@anissa111 anissa111 merged commit 2780913 into NCAR:main Apr 22, 2024
15 checks passed
@cyschneck cyschneck deleted the deprecation_477 branch April 22, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation testing Issue related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce DeprecationWarnings in Testing
4 participants