Skip to content

fix stacklevel of hdi warning #1730

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

Merged

Conversation

MarcoGorelli
Copy link
Contributor

Description

Checklist

  • Follows official PR format
  • Includes a sample plot to visually illustrate the changes (only for plot-related functions)
  • New features are properly documented (with an example if appropriate)?
  • Includes new or updated tests to cover the new feature
  • Code style correct (follows pylint and black guidelines)
  • Changes are listed in changelog

On main, this warning shows the line number from ArviZ code. On this branch, it'll show the line number from the user's code.

I wanted to add a test, but running

pytest arviz/tests/base_tests/test_stats.py -k test_hdp_2darray -W error

doesn't throw any errors, neither here nor on main - do the warnings get silenced somewhere during tests?

It shows the warning if I just execute the code:

>>> import numpy as np
>>> import arviz
>>> arviz.hdi(np.random.randn(2, 2))
<stdin>:1: FutureWarning: hdi currently interprets 2d data as (draw, shape) but this will change in a future release to (chain, draw) for coherence with other functions
array([[-1.85791612,  1.28256026],
       [ 1.23431573,  1.91356814]])
>>> 

@OriolAbril
Copy link
Member

doesn't throw any errors, neither here nor on main - do the warnings get silenced somewhere during tests?

I think it could be due to https://github.com/arviz-devs/arviz/blob/main/pytest.ini, we do have tests that use pytest.warns though, and those work.

@ahartikainen
Copy link
Contributor

When working locally, try -s tag

@MarcoGorelli
Copy link
Contributor Author

MarcoGorelli commented Jun 20, 2021

Thanks - indeed, using pytest.warns does work

Unfortunately, it doesn't check the stacklevel (pytest-dev/pytest#4343), but to demo the difference:

(venv) marco@marco-Predator-PH315-52:~/arviz-dev$ git checkout main
Switched to branch 'main'
Your branch is up-to-date with 'upstream/main'.
(venv) marco@marco-Predator-PH315-52:~/arviz-dev$ cat t.py 
import arviz as az
import numpy as np

az.hdi(np.random.randn(2, 2))
(venv) marco@marco-Predator-PH315-52:~/arviz-dev$ python t.py 
/home/marco/arviz-dev/arviz/stats/stats.py:494: FutureWarning: hdi currently interprets 2d data as (draw, shape) but this will change in a future release to (chain, draw) for coherence with other functions
  warnings.warn(
(venv) marco@marco-Predator-PH315-52:~/arviz-dev$ git checkout fix-stacklevel-of-hdi-warning 
Switched to branch 'fix-stacklevel-of-hdi-warning'
Your branch is up-to-date with 'origin/fix-stacklevel-of-hdi-warning'.
(venv) marco@marco-Predator-PH315-52:~/arviz-dev$ python t.py 
t.py:4: FutureWarning: hdi currently interprets 2d data as (draw, shape) but this will change in a future release to (chain, draw) for coherence with other functions
  az.hdi(np.random.randn(2, 2))

note how in the first case, the warning is from /home/marco/arviz-dev/arviz/stats/stats.py:494, while in the second it's from t.py:4

@codecov
Copy link

codecov bot commented Jun 20, 2021

Codecov Report

Merging #1730 (bd66f46) into main (edfbcf9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1730   +/-   ##
=======================================
  Coverage   90.86%   90.86%           
=======================================
  Files         109      109           
  Lines       11844    11844           
=======================================
  Hits        10762    10762           
  Misses       1082     1082           
Impacted Files Coverage Δ
arviz/stats/stats.py 96.60% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edfbcf9...bd66f46. Read the comment docs.

@canyon289
Copy link
Member

So should we merge this?

@OriolAbril OriolAbril merged commit 1fd8901 into arviz-devs:main Jul 27, 2021
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.

4 participants