[WIP] distribution classes #390
Conversation
|
💃 💃 |
pyglmnet/distributions.py
Outdated
| def __init__(self): | ||
| """init.""" | ||
| pass |
There was a problem hiding this comment.
you probably don't need to redefine it since you're inheriting from BaseDistribution ?
|
exciting stuff! I think it significantly improves our repository :-D I would love to see an example with |
pyglmnet/pyglmnet.py
Outdated
| if isinstance(distr, BaseDistribution): | ||
| return distr | ||
|
|
||
| elif distr == 'gaussian': |
There was a problem hiding this comment.
I wonder if we can change this huge if-else to something in which we dynamically instantiate the correct class given the string. I was thinking of something like this https://softwareengineering.stackexchange.com/a/351394, in which we have a dictionary which is holding all the required classes.
| from pyglmnet.distributions import Binomial | ||
|
|
||
|
|
||
| class CustomBinomial(Binomial): |
There was a problem hiding this comment.
@pavanramkumar I'm not sure this is legit. The log_likelihood function for Binomial does not call self.mu or does it? Is the log likelihood the log likelihood taking into account the link function?
|
Quick question. If I wanted to contribute on this one here, should I fork your repo @pavanramkumar or do you know if there is any way to push directly to this PR? |
|
I think forking might be the easiest. But maybe @pavanramkumar should confirm first if he has any local changes that have not been pushed? |
|
Thanks for picking this up @geektoni ! Please go ahead and fork. |
closes #350
TODOs
pyglmnet.py(currently commented out)simulate_glm()functionality intosimulate()methods of respective classespseudo_R2,deviance)