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.

Sign up for GitHub

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

Jump to bottom

Fix issue with included Stan files #553

Merged
merged 3 commits into from Sep 15, 2021
Merged

Fix issue with included Stan files #553

merged 3 commits into from Sep 15, 2021

Conversation

rok-cesnovar
Copy link
Member

@rok-cesnovar rok-cesnovar commented Sep 14, 2021 •
edited

Summary

Fixes #550

I want to rework this later so the --info stanc3 is not called more than once, but this fixes the issue for now, even if its a bit verbose solution.

Copyright and Licensing

Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Rok Češnovar

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

@jgabry
Copy link
Member

jgabry commented Sep 14, 2021

I want to rework this later so the --info stanc3 is not called more than once, but this fixes the issue for now, even if its a bit verbose solution.

Thanks! It's good to have a short term solution even if a bit verbose. Looks good, but is there a reason there are lots of tests commented out?

@rok-cesnovar
Copy link
Member Author

I forgot to uncomment them back…

Not at a computer right now, so I can fix that in the morning if the rest looks good.

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2021 •
edited

Codecov Report

Merging #553 (4bbe4c3) into master (1144409) will decrease coverage by 1.11%.
The diff coverage is 100.00%.

❗ Current head 4bbe4c3 differs from pull request most recent head 09b5f03. Consider uploading reports for the commit 09b5f03 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #553      +/-   ##
==========================================
- Coverage   92.82%   91.71%   -1.12%     
==========================================
  Files          12       12              
  Lines        3122     3089      -33     
==========================================
- Hits         2898     2833      -65     
- Misses        224      256      +32
Impacted Files Coverage Δ
R/args.R 98.63% <100.00%> (ø)
R/data.R 99.35% <100.00%> (ø)
R/model.R 91.49% <100.00%> (-0.59%) ⬇️
R/install.R 63.27% <0.00%> (-5.56%) ⬇️
R/run.R 94.07% <0.00%> (-1.65%) ⬇️
R/utils.R 88.88% <0.00%> (-1.39%) ⬇️
R/csv.R 98.21% <0.00%> (-0.45%) ⬇️

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 1144409...09b5f03. Read the comment docs.

@jgabry
Copy link
Member

jgabry commented Sep 14, 2021 via email

@rok-cesnovar rok-cesnovar merged commit e78a31b into master Sep 15, 2021
@rok-cesnovar rok-cesnovar deleted the fix_included_path branch September 15, 2021 11:38
@rok-cesnovar rok-cesnovar mentioned this pull request Sep 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cmdstanr model cannot find include file that exists in path
3 participants
@rok-cesnovar @jgabry @codecov-commenter