Skip to content

Fix broken VI test (pytest issue) #3779

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
merged 2 commits into from
Jan 20, 2020
Merged

Fix broken VI test (pytest issue) #3779

merged 2 commits into from
Jan 20, 2020

Conversation

ferrine
Copy link
Member

@ferrine ferrine commented Jan 20, 2020

One test is mentioned to be broken in #3775, #3776 is created to remove the test, but it would be a temporary solution.

@rpgoldman
Copy link
Contributor

@ferrine Unfortunately, fixing this for the benefit of test_variational_inference breaks it in the test file that's the source of the fixture.

I think an easier fix might be to give up on figuring out the scoping, and just copy the fixture from the source file into test_variational_inference. This is unsatisfactory, but fighting pytest is unsatisfactory, too.

@ferrine
Copy link
Member Author

ferrine commented Jan 20, 2020

I'll do that then

@ferrine
Copy link
Member Author

ferrine commented Jan 20, 2020

I think that this may happen due to module scoping

@codecov
Copy link

codecov bot commented Jan 20, 2020

Codecov Report

Merging #3779 into master will increase coverage by 6.59%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3779      +/-   ##
==========================================
+ Coverage   83.65%   90.25%   +6.59%     
==========================================
  Files         133      133              
  Lines       20336    20670     +334     
==========================================
+ Hits        17013    18656    +1643     
+ Misses       3323     2014    -1309
Impacted Files Coverage Δ
pymc3/tests/test_shape_handling.py 100% <100%> (+100%) ⬆️
pymc3/tests/test_variational_inference.py 97.77% <100%> (+97.77%) ⬆️
pymc3/sampling.py 77.9% <0%> (-5.22%) ⬇️
pymc3/model.py 89.17% <0%> (+0.12%) ⬆️
pymc3/tests/helpers.py 56.6% <0%> (+1.88%) ⬆️
pymc3/step_methods/hmc/hmc.py 96.66% <0%> (+3.33%) ⬆️
pymc3/distributions/shape_utils.py 100% <0%> (+11.11%) ⬆️
pymc3/memoize.py 96.15% <0%> (+13.46%) ⬆️
pymc3/variational/opvi.py 85.29% <0%> (+17.28%) ⬆️
pymc3/variational/callbacks.py 95.91% <0%> (+18.36%) ⬆️
... and 10 more

@lucianopaz
Copy link
Member

Great @ferrine! Looks like you fixed this! For some reason we seem to have to CI builds running in the PR. Maybe we are in the situation described here? I'm in my cellphone and can't take a closer look into that but maybe it would be worth opening an issue

@lucianopaz lucianopaz merged commit 03a64aa into master Jan 20, 2020
@rpgoldman rpgoldman deleted the disable-broken-test branch January 20, 2020 20:19
@ferrine
Copy link
Member Author

ferrine commented Jan 21, 2020

@ferrine! Looks like you fixed this!

Lol, I did not think I did it and tried to find my PR for 5 mins

@ferrine
Copy link
Member Author

ferrine commented Jan 21, 2020

I see some pytest issues on this pytest-dev/pytest#6497

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.

3 participants