-
Notifications
You must be signed in to change notification settings - Fork 101
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
Deprecate redefinition of np.testing.assert_allclose
#784
base: main
Are you sure you want to change the base?
Deprecate redefinition of np.testing.assert_allclose
#784
Conversation
94bdd55
to
ba18c65
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #784 +/- ##
==========================================
- Coverage 80.97% 80.96% -0.02%
==========================================
Files 169 169
Lines 47015 47011 -4
Branches 11497 11497
==========================================
- Hits 38072 38061 -11
- Misses 6728 6729 +1
- Partials 2215 2221 +6
|
135ff93
to
87c6ee6
Compare
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.
Asking whether we can remove one more helper from the non-test codebase
atol_, rtol_ = pytensor.tensor.math._get_atol_rtol(a, b) | ||
if rtol is not None: | ||
rtol_ = rtol | ||
if atol is not None: | ||
atol_ = atol |
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.
Coverage says this is not being used, can we just remove?
@@ -85,7 +85,15 @@ def test_variable_only(self): | |||
z = dot(x, y) | |||
assert hasattr(z.tag, "test_value") | |||
f = pytensor.function([x, y], z) | |||
assert _allclose(f(x.tag.test_value, y.tag.test_value), z.tag.test_value) | |||
atol_, rtol_ = _get_atol_rtol( |
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.
If this ends up only being used in the tests here, we can remove from math and include only in the test file?
So what is to be done here. Here's what is changing. So originally in |
87c6ee6
to
81a97df
Compare
81a97df
to
26782d2
Compare
Hi @ricardoV94 |
Description
Replaced all instances of
assert_allclose
with correspondingnp.testing.assert_allclose
.Related Issue
np.testing.assert_allclose
#551Checklist
Type of change