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

Return InferenceData by default #4744

Merged

Conversation

michaelosthege
Copy link
Member

@michaelosthege michaelosthege commented Jun 6, 2021

Also removes some unnecessary XFAIL marks.

In multiple places I edited code examples in docstrings/documentation, as well as variable names in the tests to reflect that the returned object is no longer a MultiTrace, but an InferenceData instead.

Closes #4372, #4740

  • what are the (breaking) changes that this PR makes? pm.sample now returns InferenceData by default. This was prominently announced since at least version 3.11.0.
  • important background, or details about the implementation Some tests remain with return_inferencedata=False, because they test internal logic that is relevant before the conversion. Some more tests remain with return_inferencedata=False because I was too lazy to migrate them all.
  • are the changes—especially new features—covered by tests and docstrings? Countless times.
  • linting/style checks have been run
  • consider adding/updating relevant example notebooks Many use InferenceData already.
  • right before it's ready to merge, mention the PR in the RELEASE-NOTES.md done

trace = pm.sample(100, cores=1)

samples = 500
size = 100
Copy link
Member Author

Choose a reason for hiding this comment

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

This test looks suspiciously like some asserts were accidentally deleted at some point in the past.
This is why I prefer to put a pass at the end of tests!

@michaelosthege michaelosthege force-pushed the inferencedata-by-default branch from d6e98da to ca3c236 Compare June 6, 2021 16:03
@michaelosthege michaelosthege added this to the vNext (4.0.0) milestone Jun 6, 2021
pymc3/distributions/discrete.py Outdated Show resolved Hide resolved
pymc3/distributions/discrete.py Outdated Show resolved Hide resolved
pymc3/tests/test_sampling.py Show resolved Hide resolved
pymc3/tests/test_step.py Outdated Show resolved Hide resolved
@michaelosthege michaelosthege force-pushed the inferencedata-by-default branch from ca3c236 to e0d261d Compare June 7, 2021 10:04
Also removes some unnecessary XFAIL marks.

Closes pymc-devs#4372, pymc-devs#4740

Co-authored-by: Oriol Abril <oriol.abril.pla@gmail.com>
@michaelosthege michaelosthege force-pushed the inferencedata-by-default branch from e0d261d to a584af1 Compare June 7, 2021 10:06
@michaelosthege michaelosthege merged commit 0923d25 into pymc-devs:main Jun 7, 2021
@michaelosthege michaelosthege deleted the inferencedata-by-default branch June 7, 2021 11:14
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.

Make pm.sample return InferenceData by default
3 participants