Skip to content

Remove deprecated functionality #663

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
Merged
merged 2 commits into from
Nov 16, 2022
Merged

Remove deprecated functionality #663

merged 2 commits into from
Nov 16, 2022

Conversation

kbattocchi
Copy link
Collaborator

No description provided.

@kbattocchi kbattocchi force-pushed the kebatt/removeDeprecated branch 2 times, most recently from 3f091e8 to 85f2226 Compare August 13, 2022 11:56
@kbattocchi kbattocchi force-pushed the kebatt/removeDeprecated branch 2 times, most recently from 806cf02 to 7b958d9 Compare November 2, 2022 16:08
@kbattocchi kbattocchi requested a review from fverac November 2, 2022 16:08
@fverac
Copy link
Collaborator

fverac commented Nov 2, 2022

NonparametricTwoStageLeastSquares is still referenced a few times (once in Detailed Estimator Comparison and once in DPolynomialFeatures)
https://github.com/microsoft/EconML/blob/kebatt/removeDeprecated/doc/spec/comparison.rst#id3
econml\iv\sieve_tsls.py line 93

This may also be the case for other deprecated classes, I just noticed this one.

I would have made a comment on the exact lines themselves but I'm not sure github allows that currently.

@fverac
Copy link
Collaborator

fverac commented Nov 2, 2022

Also references to old classes ForestDML here:
https://github.com/microsoft/EconML/blob/kebatt/removeDeprecated/notebooks/ForestLearners%20Basic%20Example.ipynb

@fverac
Copy link
Collaborator

fverac commented Nov 2, 2022

Reference to old ContinuousTreatmentOrthoForest in https://github.com/microsoft/EconML/blob/kebatt/removeDeprecated/notebooks/Causal%20Forest%20and%20Orthogonal%20Random%20Forest%20Examples.ipynb

@fverac
Copy link
Collaborator

fverac commented Nov 2, 2022

Reference to old DiscreteTreatmentOrthoForest in docs at
https://github.com/microsoft/EconML/blob/kebatt/removeDeprecated/doc/spec/estimation/dr.rst

@kbattocchi kbattocchi force-pushed the kebatt/removeDeprecated branch 4 times, most recently from b822d23 to 2ef37f5 Compare November 9, 2022 16:04
@kbattocchi kbattocchi force-pushed the kebatt/removeDeprecated branch 2 times, most recently from 6f2ffee to a62aaa0 Compare November 15, 2022 21:03
@kbattocchi kbattocchi force-pushed the kebatt/removeDeprecated branch 3 times, most recently from 01a17ea to 6a22136 Compare November 16, 2022 12:38
@kbattocchi kbattocchi force-pushed the kebatt/removeDeprecated branch from 6a22136 to 1d3d848 Compare November 16, 2022 13:25
@kbattocchi kbattocchi mentioned this pull request Nov 16, 2022
Closed
@kbattocchi
Copy link
Collaborator Author

@fverac I've addressed your issues, please review

@kbattocchi kbattocchi merged commit 03f53c5 into main Nov 16, 2022
@kbattocchi kbattocchi deleted the kebatt/removeDeprecated branch November 16, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fverac fverac fverac approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kbattocchi @fverac