-
Notifications
You must be signed in to change notification settings - Fork 65
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
Enable summary
method for all currently implemented frequentist experiments
#355
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #355 +/- ##
==========================================
+ Coverage 79.96% 80.87% +0.91%
==========================================
Files 21 21
Lines 1642 1668 +26
==========================================
+ Hits 1313 1349 +36
+ Misses 329 319 -10 ☔ View full report in Codecov by Sentry. |
View / edit / reply to this conversation on ReviewNB juanitorduz commented on 2024-06-19T08:00:38Z These numbers look in different columns. Why don't er print a dataframe? |
Some tables based on |
Very good point @juanitorduz . For the moment I've kept the same basic approach, but have improved the formatting. The function that prints the model coefficients now evaluates the length of the longest coefficient label so that it can align the coefficient values appropriately. I'm very open to rethinking the nature of the summary outputs, but I'm tempted to deal with that in a separate issue. For example, there is already #174 as one option. I'll do a similar formatting improvement for the pymc experiments in a different issue now. What do you think? PS. I just noticed I probably have to update the doctests 🤣 Will do that now :) |
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.
ok! let's follow up the next iteration on the associated issue :)
summary
method on all the existing frequentist experimentsresult.summary()
functionalitysummary
method just to get code coverage to pass. But note that Have more coherent testing of thesummary
method #305 exists to do better testing on this front