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 PrePostNEGD input validation #247

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

jpreszler
Copy link
Contributor

@jpreszler jpreszler commented Sep 21, 2023

This resolves issue #161

I tried the PrePostNEGD example with group being binary (current), boolean, or categorical with levels 'A' and 'B'. All three pass the _series_has_2_levels check but in the prediction after model fitting, errors result in the 'A'/'B' case because 'untreated' is given values of 0 (and 'treated' 1). Switching to _is_variable_dummy_coded moves the error to a data exception in the input validation, which makes more sense.

A more complicated fix would be to assign the two levels in lines 1059 and 1070 rather than 0's and 1's, but then we would need to know which is the 'treatment' level so probably best to leave those transformations to users.

I also fixed a bug that requires the group variable to be called 'group' and adjusted some re-used variable names whose names didn't match what was being stored.

@jpreszler jpreszler force-pushed the issue_161_prepostnegd_inputs branch from 59e55b1 to 526d8a8 Compare September 21, 2023 22:55
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #247 (526d8a8) into main (133b987) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #247   +/-   ##
=======================================
  Coverage   73.89%   73.89%           
=======================================
  Files          19       19           
  Lines        1149     1149           
=======================================
  Hits          849      849           
  Misses        300      300           
Files Changed Coverage Δ
causalpy/pymc_experiments.py 66.75% <100.00%> (ø)

@jpreszler
Copy link
Contributor Author

@drbenvincent also ready when you get a chance

@drbenvincent drbenvincent added the enhancement New feature or request label Oct 17, 2023
@drbenvincent drbenvincent merged commit adea6f7 into pymc-labs:main Oct 17, 2023
@drbenvincent drbenvincent added bug Something isn't working and removed enhancement New feature or request labels Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants