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

enabled type checking for io_pymc3 input #1629

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

MgeeeeK
Copy link
Contributor

@MgeeeeK MgeeeeK commented Mar 24, 2021

Description

Enabled type checking for io_pymc3 input... Now it raises an error whenever input type is of InferenceData.

Fix #1617

Checklist

  • Follows official PR format
  • Includes new or updated tests to cover the new feature
  • Code style correct (follows pylint and black guidelines)
  • Changes are listed in changelog

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

looks good, can you also add a test for this?

@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #1629 (b28472b) into main (7ec650f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head b28472b differs from pull request most recent head 9f468ee. Consider uploading reports for the commit 9f468ee to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1629   +/-   ##
=======================================
  Coverage   90.92%   90.92%           
=======================================
  Files         108      108           
  Lines       11663    11665    +2     
=======================================
+ Hits        10604    10606    +2     
  Misses       1059     1059           
Impacted Files Coverage Δ
arviz/data/io_pymc3.py 90.75% <100.00%> (+0.06%) ⬆️

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 7ec650f...9f468ee. Read the comment docs.

@MgeeeeK MgeeeeK requested a review from OriolAbril March 24, 2021 18:47
CHANGELOG.md Outdated
@@ -25,6 +25,7 @@
* Fix `ess/rhat` plots in `plot_forest` ([1606](https://github.com/arviz-devs/arviz/pull/1606))
* Fix `from_numpyro` crash when importing model with `thinning=x` for `x > 1` ([1619](https://github.com/arviz-devs/arviz/pull/1619))
* Upload updated mypy.ini in ci if mypy copilot fails ([1624](https://github.com/arviz-devs/arviz/pull/1624))
* Enforced using `pymc3.backends.base.MultiTrace` as the `trace` value for `io_pymc3` ([1629](https://github.com/arviz-devs/arviz/pull/1629))
Copy link
Member

Choose a reason for hiding this comment

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

I would say something along the lines of raising an informative error if inferencedata is passed

@@ -139,6 +139,16 @@ def test_from_pymc_predictions(self, data, eight_schools_params):
assert ivalues.shape[0] == 1 # one chain in predictions
assert np.all(np.isclose(ivalues[0], values))

def test_from_pymc_trace_inference_data(self):
# check if error is raised successfully after passing InferenceData as trace
Copy link
Member

Choose a reason for hiding this comment

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

This should be a docstring using """ instead of a comment

Comment on lines 144 to 148
with pm.Model():
p = pm.Uniform("p", 0, 1)
pm.Binomial("w", p=p, n=2, observed=1)
trace = pm.sample(100, chains=2, return_inferencedata=True)
assert isinstance(trace, InferenceData)
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need to sample from a model, we can use an example inferencedata or create it with from_dict and then do az.from_pymc3(idata, model=pm.Model()) directly

@OriolAbril OriolAbril merged commit a9917a2 into arviz-devs:main Mar 26, 2021
@MgeeeeK MgeeeeK deleted the input_check_io_pymc3 branch March 30, 2021 02:29
utkarsh-maheshwari pushed a commit to utkarsh-maheshwari/arviz that referenced this pull request May 27, 2021
* enabled type checking for io_pymc3 input

* Added test and updated changelog

* Fixed test and updated changelog msg
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.

Check for InferenceData input on from_pymc3
2 participants