Skip to content

With Cmdstan 2.22+ use JSON by default#110

Merged
jgabry merged 5 commits intomasterfrom
revert-100-stan_rdump
Feb 17, 2020
Merged

With Cmdstan 2.22+ use JSON by default#110
jgabry merged 5 commits intomasterfrom
revert-100-stan_rdump

Conversation

@rok-cesnovar
Copy link
Member

@rok-cesnovar rok-cesnovar commented Jan 16, 2020

Reverts #100 since the underlying cmdstan issue will be resolved for 2.22

Closes #109

@rok-cesnovar rok-cesnovar changed the title Revert "Switch back to using Rdata temporarily while issue with 0x0s gets resolved for JSON" With Cmdstan 2.22+ use JSON by default Feb 3, 2020
@codecov-io
Copy link

codecov-io commented Feb 6, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@4845369). Click here to learn what that means.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #110   +/-   ##
=========================================
  Coverage          ?   92.49%           
=========================================
  Files             ?        7           
  Lines             ?     1186           
  Branches          ?        0           
=========================================
  Hits              ?     1097           
  Misses            ?       89           
  Partials          ?        0
Impacted Files Coverage Δ
R/model.R 97.05% <60%> (ø)
R/args.R 98.8% <66.66%> (ø)
R/path.R 95.65% <66.66%> (ø)

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 4845369...434b2ba. Read the comment docs.

@rok-cesnovar
Copy link
Member Author

This is ready for review. Codecov on appveyor seems to not be working, so I decided to comment it out until we sort out the reason.

Copy link
Member

@jgabry jgabry left a comment

Choose a reason for hiding this comment

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

Thanks, looks good! Just one addition requested.

@rok-cesnovar
Copy link
Member Author

Appveyor tests pass but for some reason the codecov tests fail. I am not sure why.

@jgabry
Copy link
Member

jgabry commented Feb 17, 2020

I don't know why codecov is failing either, but this looks good. I'll merge it now.

@jgabry jgabry merged commit e5d011a into master Feb 17, 2020
@jgabry jgabry deleted the revert-100-stan_rdump branch February 17, 2020 22:16
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.34%. Comparing base (44e1d5c) to head (856285e).
⚠️ Report is 2898 commits behind head on master.

Files with missing lines Patch % Lines
R/model.R 42.85% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage   92.64%   92.34%   -0.31%     
==========================================
  Files           7        7              
  Lines        1183     1188       +5     
==========================================
+ Hits         1096     1097       +1     
- Misses         87       91       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Make JSON input the default option

4 participants