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

Add tests from pyroapi #396

Merged
merged 6 commits into from
Oct 12, 2019
Merged

Add tests from pyroapi #396

merged 6 commits into from
Oct 12, 2019

Conversation

neerajprad
Copy link
Member

Addresses pyro-ppl/pyro-api#2.

fehiepsi
fehiepsi previously approved these changes Oct 10, 2019
Copy link
Member

@fehiepsi fehiepsi left a comment

Choose a reason for hiding this comment

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

LGTM! I just have a nit. Pending further comments from @fritzo .

numpyro/compat/pyro.py Outdated Show resolved Hide resolved
'test': [
'flake8',
'pytest>=4.1',
'pyro-api@https://api.github.com/repos/pyro-ppl/pyro-api/tarball/master'
Copy link
Member

@fritzo fritzo Oct 11, 2019

Choose a reason for hiding this comment

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

👍 I'll use this method in funsor as well: pyro-ppl/funsor#267

test/conftest.py Outdated Show resolved Hide resolved
@@ -0,0 +1,7 @@
class UnsupportedAPIWarning(Warning):
Copy link
Member

Choose a reason for hiding this comment

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

Looks great! If this class proves useful, we may want to move it to pyroapi, maybe calling it a NotImplementedWarning to match NotImplementedError.

test/test_pyroapi.py Outdated Show resolved Hide resolved
fritzo
fritzo previously approved these changes Oct 11, 2019
fritzo
fritzo previously approved these changes Oct 11, 2019
@fritzo
Copy link
Member

fritzo commented Oct 11, 2019

Should we merge this before pyro-ppl/pyro-api#6, or
Do you want to remove the seed logic in this PR?

@fritzo
Copy link
Member

fritzo commented Oct 12, 2019

Can we merge this to unblock pyro-ppl/pyro-api#5 ?

@neerajprad neerajprad merged commit 9790801 into master Oct 12, 2019
@neerajprad
Copy link
Member Author

Can we merge this to unblock pyro-ppl/pyro-api#5 ?

Done!

@neerajprad neerajprad deleted the pyroapi branch November 19, 2019 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants