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

from_cmdstanpy - fix bug introduced by refactor (PR 1558) #1564

Merged
merged 13 commits into from
Feb 16, 2021

Conversation

mitzimorris
Copy link
Contributor

Description

Refactor of from_cmdstanpy failed to account for difference in xarray data shape for scalar and multi-dim vars;
need to squeeze scalar vars to single column.

Checklist

@codecov
Copy link

codecov bot commented Feb 14, 2021

Codecov Report

Merging #1564 (f69a024) into main (f4606e4) will increase coverage by 0.00%.
The diff coverage is 87.50%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1564   +/-   ##
=======================================
  Coverage   90.27%   90.28%           
=======================================
  Files         105      105           
  Lines       11397    11403    +6     
=======================================
+ Hits        10289    10295    +6     
  Misses       1108     1108           
Impacted Files Coverage Δ
arviz/data/io_cmdstanpy.py 61.51% <87.50%> (+0.81%) ⬆️
arviz/plots/posteriorplot.py 90.00% <0.00%> (ø)

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 f4606e4...f69a024. Read the comment docs.

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 great, thanks!

The documentation build is failing, I think it's due to the latest release of myst-parser, we should pin it to the previous version until it's fixed.

@mitzimorris
Copy link
Contributor Author

why did PR pass before I added shape checks?
did release of parser happen between time of first PR and when I added more checks?

@OriolAbril
Copy link
Member

The check that failed is the one that makes sure the documentation can be generated, it has no relation to this PR. The checks about the shape did pass (they are evaluated in ExternalTests job, the one that failed is BuildDocs), sorry about that. I have just checked and something must have gone wrong with their release process. They released 0.13.4 right in between these changes above but 0.13.5 is already available and .4 has been yanked from pypi: https://pypi.org/project/myst-parser/#history

Adding the shape checks again now should work and all ci tests should pass

@mitzimorris
Copy link
Contributor Author

tests are failing pylint: E1133, E1101. - pystan and base_tests

/usr/bin/bash --noprofile --norc /home/vsts/work/_temp/4cb9bbfc-22f9-4d96-9fb1-f14175bd8e16.sh
************* Module arviz.data.io_pystan
arviz/data/io_pystan.py:245:23: E1133: Non-iterable value names is used in an iterating context (not-an-iterable)
arviz/data/io_pystan.py:465:23: E1133: Non-iterable value names is used in an iterating context (not-an-iterable)
************* Module arviz.tests.base_tests.test_rcparams
arviz/tests/base_tests/test_rcparams.py:268:8: E1101: Instance of 'dict' has no 'posterior' member (no-member)
arviz/tests/base_tests/test_rcparams.py:268:8: E1101: Instance of 'InferenceData' has no 'posterior' member (no-member)
arviz/tests/base_tests/test_rcparams.py:275:8: E1101: Instance of 'InferenceData' has no 'posterior' member (no-member)
arviz/tests/base_tests/test_rcparams.py:275:8: E1101: Instance of 'dict' has no 'posterior' member (no-member)


.pylintrc Outdated Show resolved Hide resolved
@OriolAbril OriolAbril merged commit 1a7f83f into main Feb 16, 2021
@OriolAbril OriolAbril deleted the update/from_cmdstanpy_patch branch February 16, 2021 05:10
utkarsh-maheshwari pushed a commit to utkarsh-maheshwari/arviz that referenced this pull request May 27, 2021
…#1564)

* squeezing scalar vars

* added checks on variable shape

* lint fix

* remove checks on variable shape

* adding back shape checks

* pylint - diable no-member, not-an-iterable

* black

* Update .pylintrc indentation

* add commas

Co-authored-by: Oriol (ZBook) <[email protected]>
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.

2 participants