Skip to content

Bugfix/455 csv single row#458

Merged
WardBrian merged 6 commits intodevelopfrom
bugfix/455-CSV-single-row
Sep 10, 2021
Merged

Bugfix/455 csv single row#458
WardBrian merged 6 commits intodevelopfrom
bugfix/455-CSV-single-row

Conversation

@mitzimorris
Copy link
Copy Markdown
Member

@mitzimorris mitzimorris commented Sep 10, 2021

Submission Checklist

  • Run unit tests
  • Declare copyright holder and open-source license: see below

Summary

Adds more unit tests to bugfix from ahartikainen:bugfix/shape_issues; also fixes bug in CmdStanGQ - method stan_variable fails to properly reshape container vars.

Swapped out test model test/data/shape.stan with model test/data/matrix_var.stan which is bernoulli model with data supplied via transformed data, and generated quantities block which creates 4 x 3 matrix z where i,j entry == row_num.

Unit tests check that for all methods, z has both correct shape and correct values.

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): Columbia University

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

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 10, 2021

Codecov Report

Merging #458 (55fbc05) into develop (3224542) will increase coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #458      +/-   ##
===========================================
+ Coverage    76.36%   76.54%   +0.17%     
===========================================
  Files           27       27              
  Lines         8979     9135     +156     
===========================================
+ Hits          6857     6992     +135     
- Misses        2122     2143      +21     
Impacted Files Coverage Δ
cmdstanpy/cmdstanpy/cmdstan_args.py 94.71% <0.00%> (-0.53%) ⬇️
...work/cmdstanpy/cmdstanpy/cmdstanpy/cmdstan_args.py 94.71% <0.00%> (-0.53%) ⬇️
a/cmdstanpy/cmdstanpy/cmdstanpy/cmdstan_args.py 93.90% <0.00%> (-0.47%) ⬇️
cmdstanpy/cmdstanpy/model.py 78.38% <0.00%> (ø)
a/cmdstanpy/cmdstanpy/cmdstanpy/model.py 80.72% <0.00%> (ø)
...runner/work/cmdstanpy/cmdstanpy/cmdstanpy/model.py 78.38% <0.00%> (ø)
cmdstanpy/cmdstanpy/stanfit.py 91.91% <0.00%> (+0.27%) ⬆️
...nner/work/cmdstanpy/cmdstanpy/cmdstanpy/stanfit.py 91.91% <0.00%> (+0.27%) ⬆️
a/cmdstanpy/cmdstanpy/cmdstanpy/stanfit.py 91.70% <0.00%> (+0.28%) ⬆️
cmdstanpy/cmdstanpy/__init__.py 86.95% <0.00%> (+0.59%) ⬆️
... and 5 more

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 3224542...55fbc05. Read the comment docs.

Copy link
Copy Markdown
Contributor

@ahartikainen ahartikainen left a comment

Choose a reason for hiding this comment

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

LGTM

@WardBrian
Copy link
Copy Markdown
Member

More of an aesthetic comment than anything, but I don't think the name test_bug_455 is particularly helpful. Could we call it something like test_single_row_csv_reshaping? A mouthful, but doesn't require you to open a browser or try to reverse engineer the bug to figure out what it is testing.

Other than that, LGTM

@WardBrian WardBrian linked an issue Sep 10, 2021 that may be closed by this pull request
Copy link
Copy Markdown
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

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

Good to merge once tests past

@WardBrian WardBrian merged commit 84f9745 into develop Sep 10, 2021
@dharris10-chwy
Copy link
Copy Markdown

Thanks again everyone!

@mitzimorris mitzimorris deleted the bugfix/455-CSV-single-row branch September 17, 2021 19:15
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.

unexpected output for parameter matrices

5 participants