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

Fix nan handling and fix n_eff to ess #573

Merged
merged 3 commits into from
Jan 29, 2019
Merged

Conversation

ahartikainen
Copy link
Contributor

@ahartikainen ahartikainen commented Jan 28, 2019

This PR makes ess to return float if NaN is seen.

Until there exists a way to return integer dtype with NaN, this is the best option.

Let's follow how xarray evolves with this issue. pydata/xarray#1194

Also I noticed that there was many n_eff left so I changed them to ess.

@ahartikainen
Copy link
Contributor Author

Actually we return dataframe by default, but still we have an option to return xarray, so I think we should follow what xarray does.

@ahartikainen ahartikainen changed the title Fix stats to pass through nans Fix nan handling and fix n_eff to ess Jan 28, 2019
@aloctavodia
Copy link
Contributor

LGTM!

@@ -29,7 +29,7 @@ def plot_forest(
credible_interval=0.94,
rope=None,
quartiles=True,
eff_n=False,
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for catching this

@canyon289
Copy link
Member

canyon289 commented Jan 28, 2019

LGTM, I don't know what is going on with coveralls, it seems to be including linter, It should only be including unit test containers

@ahartikainen
Copy link
Contributor Author

Yeah, it fails sometimes. I will restart one of the builds (e.g. PR) and let's see how that goes. (Inside coveralls is shows 100%)

Linter one tests --save.

@ahartikainen ahartikainen merged commit 055a9ec into master Jan 29, 2019
@ahartikainen ahartikainen deleted the bugfix/nan_issue branch January 29, 2019 11:52
@ColCarroll ColCarroll mentioned this pull request Feb 22, 2019
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