OLS

[mortonjt]

Depends on #32

[coveralls]

Coverage decreased (-1.4%) to 97.984% when pulling defa170 on mortonjt:ols into 356896d on biocore:regression_results.

[coveralls]

Coverage decreased (-1.6%) to 97.976% when pulling e5b463d on mortonjt:ols into ddc3d40 on biocore:regression_results.

[coveralls]

Coverage increased (+0.06%) to 99.585% when pulling 67b937e on mortonjt:ols into ddc3d40 on biocore:regression_results.

[wasade]

doc string?

[wasade]

minor comments, 👍 otherwise

[ElDeveloper]

Should tree here be _tree?

[mortonjt]

Good catch. Thanks!

[coveralls]

Coverage increased (+0.06%) to 99.592% when pulling ed81fc6 on mortonjt:ols into ddc3d40 on biocore:regression_results.

[coveralls]

Coverage increased (+0.06%) to 99.592% when pulling c228a46 on mortonjt:ols into ddc3d40 on biocore:regression_results.

[mortonjt]

Thanks for the speedy feedback!

I had just one question concerning the user interface. I updated the docstring in ols to give a bit more context.

When users typically specify regressions in R, they specify something like as follows

y ~ x1 + x2 + x3

where y is the response variable, and x1, x2 and x3 are the explanatory variables (i.e. environmental variables).

Now in this case, we are running a linear regression on each balance. So if there are D species, we run a regression on D-1 balances.

For example, if we had 3 balances, the regression equations would look like as follows

y1 ~ x1 + x2 + x3
y2 ~ x1 + x2 + x3
y2~ x1 + x2 + x3

Now, this isn't practical if there are 1000 of balances - you cannot ask the user to manually input 1000 regression formulas. Which is why that I knocked out the response variable from the equation.

However, it may not be intuitive for the user that a regression is being run if no response variable is being run. One other idea that I had was maybe to put in a dummy variable like as follows

balance ~ x1 + x2 + x3

So that the user will know that the regression is being run directly on the balances. And have it throw an error if the response variable isn't specified correctly.

What do you guys think? Do you think this PR is ok as is, or should we modify the regression formula handling?

[wasade]

Meh, see if it actually ends up being confusing going forward and address
if needed?

On Wed, Aug 17, 2016 at 10:52 PM, Jamie Morton [email protected]
wrote:

Thanks for the speedy feedback!

I had just one question concerning the user interface. I updated the
docstring in ols to give a bit more context.

When users typically specify regressions in R, they specify something like
as follows

y ~ x1 + x2 + x3

where y is the response variable, and x1, x2 and x3 are the explanatory
variables (i.e. environmental variables).

Now in this case, we are running a linear regression on each balance. So
if there are D species, we run a regression on D-1 balances.

For example, if we had 3 balances, the regression equations would look
like as follows

y1 ~ x1 + x2 + x3
y2 ~ x1 + x2 + x3
y2~ x1 + x2 + x3

Now, this isn't practical if there are 1000 of balances - you cannot ask
the user to manually input 1000 regression formulas. Which is why that I
knocked out the response variable from the equation.

However, it may not be intuitive for the user that a regression is being
run if no response variable is being run. One other idea that I had was
maybe to put in a dummy variable like as follows

balance ~ x1 + x2 + x3

So that the user will know that the regression is being run directly on
the balances. And have it throw an error if the response variable isn't
specified correctly.

What do you guys think? Do you think this PR is ok as is, or should we
modify the regression formula handling?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#33 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAc8ssol4JscJeen0GzZ1AU8p4BOYFt8ks5qg_MJgaJpZM4JY7ox
.

[mortonjt]

👍 Right on. In that case, all of the comments have been addressed.

[coveralls]

Coverage increased (+0.06%) to 99.592% when pulling 1e7025e on mortonjt:ols into ddc3d40 on biocore:regression_results.

[antgonza]

Should this have returns, params, etc?

[antgonza]

A few comments.

[ElDeveloper]

Meh, see if it actually ends up being confusing going forward and address
if needed?

👍 .... the code looks good.

[coveralls]

Coverage increased (+0.06%) to 99.592% when pulling 55cfcb2 on mortonjt:ols into ddc3d40 on biocore:regression_results.

[mortonjt]

Comments have been addressed!

[ElDeveloper]

Thanks @mortonjt.