-
Notifications
You must be signed in to change notification settings - Fork 37
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
Restructure API and support backward compatibility #332
Conversation
dev: allow backward compatibility.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #332 +/- ##
==========================================
+ Coverage 85.79% 86.19% +0.40%
==========================================
Files 38 53 +15
Lines 3421 4043 +622
==========================================
+ Hits 2935 3485 +550
- Misses 486 558 +72 ☔ View full report in Codecov by Sentry. |
@@ -313,7 +313,7 @@ def _check_for_separation(Y: pd.DataFrame, fe: pd.DataFrame, check: str = "fe") | |||
""" | |||
Check for separation. | |||
|
|||
Check for separation of Poisson Regression. For details, see the pplmhdfe | |||
Check for separation of Poisson Regression. For details, see the ppmlhdfe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting this typo. I was convinced it was pp l m hdfe 😄
Not sure what is going on with the tests on 3.10. I will merge now nevertheless as the PR looks great. As I said before, in my experience, these errors arise occasionally and tend to go away by themselves. Though we should make sure to fix this before the next Pypi release =) Thanks for the PR Dave! |
Thanks for merging the PR! This code actually passes Python 3.10 test several hours ago, but after I made a small correction in typo, the test breaks again...Quite confusing. Hopefully we can figure out what's going on...I will take a look at this problem in the following few days. |
Hi Alex, as we discussed in #329, here is the pull request for restructuring API. It also supports existing API used in your Jupyter Notebook, with a FutureWarning. Here are a list of major changes:
__init__py
for the package and each sub-modules.estimation
module to avoid namespace conflict. For example,detect_singleton.py
contains functiondetect_singleton()
. Organic calling it will bepyfixest.estimation.detect_singleton.detect_singleton
. To avoid such ambiguity between sub-module name and function name, I renamedetect_singleton.py
todetect_singleton_.py
There are lots of files changed. The review may take lots of time. Thank you so much for considering this PR!
Best regards,
Dave