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

Set tests to error on uncaught warnings #167

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

brandonwillard
Copy link
Member

Closes #166

@brandonwillard brandonwillard added bug Something isn't working important This label is used to indicate priority over things not given this label testing labels Aug 22, 2022
@brandonwillard brandonwillard self-assigned this Aug 22, 2022
@brandonwillard brandonwillard force-pushed the catch-warnings-in-tests branch 4 times, most recently from e546179 to 47d8aad Compare August 22, 2022 22:27
@rlouf
Copy link
Member

rlouf commented Nov 18, 2022

I rebased this PR on main, the tests fail because of the following warning:

UserWarning: Rewrite warning: The Op i0 does not provide a C implementation. As well as being potentially slow, this also disables loop fusion.

i0 seems to be the modified Bessel function of the first kind of order 0 (https://github.com/aesara-devs/aesara/blob/a1739f6c56f018d3c5508cd67e0e9a35617f66d6/aesara/tensor/math.py#L1402). Do we want a C implementation for this in Aesara or should we just ignore the warning?

@brandonwillard
Copy link
Member Author

Do we want a C implementation for this in Aesara or should we just ignore the warning?

Ideally, we would have a C implementation, but we should simply catch/ignore acceptable warnings like this using pytest.warns or something similar.

@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Base: 95.16% // Head: 95.08% // Decreases project coverage by -0.08% ⚠️

Coverage data is based on head (236dcbb) compared to base (961496a).
Patch coverage: 98.11% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
- Coverage   95.16%   95.08%   -0.09%     
==========================================
  Files          12       12              
  Lines        1945     1974      +29     
  Branches      254      258       +4     
==========================================
+ Hits         1851     1877      +26     
- Misses         52       55       +3     
  Partials       42       42              
Impacted Files Coverage Δ
aeppl/transforms.py 96.25% <92.85%> (-0.19%) ⬇️
aeppl/joint_logprob.py 95.00% <100.00%> (+0.88%) ⬆️
aeppl/logprob.py 98.07% <100.00%> (+0.03%) ⬆️
aeppl/scan.py 94.73% <100.00%> (ø)
aeppl/mixture.py 96.59% <0.00%> (-1.14%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rlouf rlouf merged commit 9a1db19 into aesara-devs:main Nov 21, 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 important This label is used to indicate priority over things not given this label testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set filterwarnings to error in the tests
2 participants