Closed GStechschulte closed 1 year ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
@tomicapretto although it is ~660 lines of code added 😵💫 a lot of it comes from docstring additions, more error handling, and tests. Most of the added functionality leveraged existing functions.
@GStechschulte being 100% honest I can't understand all the changes as well as you do. I do see in many cases you made things more general and you are reusing code more, which is great. So once you finish the implementation, I trust your judgment to merge this.
Two more comments:
interpret
submodule and I think it's quite mature at this point. being 100% honest I can't understand all the changes as well as you do
Mmm. That's not a good sign in my opinion. Is it the code diff that you are unsure about, or what has changed in the predictions
functionality?
The test failure is because of some non-compatible type hint on Python 3.9. I _think it's fixed https://github.com/pymc-devs/pymc/pull/6945 but I'm not sure.
It has been resolved.
Do you think we can cut a 0.13.0 release after this? I just realized we don't have a release with the interpret submodule and I think it's quite mature at this point.
Yup, I think we can go ahead with that 👍🏼
Mmm. That's not a good sign in my opinion. Is it the code diff that you are unsure about, or what has changed in the predictions functionality?
Oh no, I don't mean this in a bad way at all. What I'm saying is that the submodule grew a lot, for good reasons, and I'm not as familiar with everything as you are. So I can only provide a high-level review without getting deep into details because it would take much more time.
Looks great! Go ahead and merge if you don't plan to add anything.
@GStechschulte, many thanks for the work on the submodule, it really helps a lot! I can report that the preliminary average_by solutions already worked very well not only on the simulated data set but also on others. The predictions align well with the observed data.
@jt-lab Thank you and this is great to hear! 😄 We / I really appreciate you taking the time to open the issues and to give feedback. Cheers!
This PR adds new functionality to the
predictions
andplot_predictions
functions ininterpret
and resolves #735. Users can nowPreviously, users could only pass a string or list of covariates to compute conditional adjusted predictions. Now,
predictions
has "most of" the functionality that {marginaleffects} has. Additionally, the changes result in a more standard API when calling comparisons, predictions, and slopes. Each function has the arg.conditional
in which the user can "condition" their estimates on. Furthermore, each function can now compute:None
)Below, you will find a couple demos:
Unit level predictions and average over
hp
andwt
to obtain marginal effects ofgear
andcyl
:Compute a pairwise grid using user-provided values and compute predictions:
To do:
plot_predictions
plot_predictions