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

Raise error if fitted propensity is passed through nuisance models #10

Merged
merged 12 commits into from
Jun 18, 2024

Conversation

FrancescMartiEscofetQC
Copy link
Contributor

This PR fixes a bug which allowed the user to pass a fitted propensity model through the fitted_nuisance_models parameter. I would suggest to not allow this to avoid having to handle multiple cases in other classes such as MetaLearnerGridSearchCV.

Checklist

  • Added a CHANGELOG.rst entry

Sorry, something went wrong.

FrancescMartiEscofetQC and others added 5 commits June 14, 2024 12:55

Verified

This commit was signed with the committer’s verified signature.
joshcooper Josh Cooper
Co-authored-by: Kevin Klein <7267523+kklein@users.noreply.github.com>
Speedup tests
Switch `strict` meaning in `validate_number_positive`
@FrancescMartiEscofetQC FrancescMartiEscofetQC marked this pull request as ready for review June 14, 2024 14:41
FrancescMartiEscofetQC and others added 3 commits June 14, 2024 16:56
Co-authored-by: Kevin Klein <7267523+kklein@users.noreply.github.com>
kklein
kklein previously approved these changes Jun 17, 2024
Copy link
Collaborator

@kklein kklein left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@kklein kklein left a comment

Choose a reason for hiding this comment

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

Should we add a test for this?

@kklein kklein merged commit a908614 into main Jun 18, 2024
6 checks passed
@kklein kklein deleted the fix_propensity_reusage branch June 18, 2024 08:39
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.

None yet

2 participants