-
Notifications
You must be signed in to change notification settings - Fork 104
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
ENH: R poly compatibility #92
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #92 +/- ##
==========================================
+ Coverage 98.96% 98.98% +0.02%
==========================================
Files 30 32 +2
Lines 5585 5703 +118
Branches 775 791 +16
==========================================
+ Hits 5527 5645 +118
Misses 35 35
Partials 23 23
Continue to review full report at Codecov.
|
I realized after writing that this may relate to #20 . But 20 points out some good questions...
Thanks. |
Is it possible for this to share the core implementation with |
I would think so. Honestly I wrote this before I noticed the above noted, Issue that was already filed. Maybe I can incorporate those thoughts into this too. |
d290dd3
to
8fa9eef
Compare
@njsmith 3 changes...
|
Also dumb question... Does |
Hmm, that's a ton of options there, between raw versus not, different polynomial bases, standardizing... I guess my first question is, are any of these... useful? Besides the boring basic R-compatible orthonormalized polynomials? Is there some compelling argument for using them in some particular situation, or an existing audience who expects them? Regarding pandas: nominally at least we still don't depend on pandas. You're right that there ought to be tests for this, though, oops. Possibly this doesn't make sense anymore; it's a different world now than it was back in 2012 or whatever when this decision was made... OTOH I dunno if people using patsy with scikit-learn for example necessarily use/want pandas. I've just tried adding a no-pandas test to the travis matrix -- I guess in ~10 minutes we'll know if (a) I'm any good at convincing travis to do what I want, (b) if so, whether the no-pandas branches actually work! |
master branch is now testing the no-pandas configuration too, so the next time you push it then codecov should start calculating your stats more accurately :-) |
Yeah I may have gone overboard there... :-/ I think QR vs. raw is necessary. Standardizing is kind of dumb... Not sure why you'd use it ever. AFAIU, the point of scaling in this way is just so that 1. the columns of data in the design matrix are relatively orthogonal and 2. when columns are of wildly different scale, the I don't really know the reasons for the differences in poly vs. chebyshev vs legende vs. laguerre. Josef poster mentioned that we should point to a generic XXXvander, I just assumed we let the user pick. I may have misinterpreted some of what Josef was trying to say. You think I should drop all the other vanders other than polyvander and standardizing? I doubt I would use anything but that. Plus, my tests are vs. R, which is just QR-orthogonalized polyvander so the rest of this we are just assuming... cc @josef-pkt - I know you're probably busy, but I'd appreciate your thoughts if there's any value in all this. Happy to remove this complexity if its not worthwhile. |
efa3cd6
to
2f9b292
Compare
I've simplified this back down so that it mimics R's |
799be8d
to
d18429a
Compare
@njsmith - Its been over a year, but I think this is close to being good enough. Any comments? Thanks! |
Travis fixes
… addition to qr decomposition. Added all numpy polynomial types. remove poly
d18429a
to
0cb862b
Compare
5a5e657
to
4d107c6
Compare
Any update on this PR? |
Is this merged yet? |
b07ba3f
to
48fd2e4
Compare
Hi @thequackdaddy ! I took a quick look through, and this looks good to me. Let me know if this is still good to merge as is. If so, I'll merge it in :). |
Support for |
4f8a70c
to
691eb4e
Compare
Hello,
I've added the function
poly
which attempts to reproduce R'spoly
function.See here: https://stat.ethz.ch/R-manual/R-devel/library/stats/html/poly.html
Thanks.