-
Notifications
You must be signed in to change notification settings - Fork 7
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
General Regression Analysis Wrapper #125
Conversation
thanks @drewrip , could you add a pydantic model to do define the regression result object? the way I prefer to generate the object looks like this: (model definition, object generation)
can you make |
matrix_benchmarking/regression.py
Outdated
if abs(z_score) > self.threshold: | ||
return RegressionStatus( | ||
1, | ||
direction=1 if z_score > 0 else -1, | ||
explanation="z-score greater than threshold", | ||
details={"threshold": self.threshold, "zscore": z_score} | ||
) | ||
else: | ||
return RegressionStatus( | ||
0, | ||
explanation="z-score not greater than threshold", | ||
details={"threshold": self.threshold, "zscore": z_score} | ||
) |
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.
can you try to have a single return here, and use variables to save the explanation/status
that will make the code more readable (=always return a RegressionStatus
, but if above threshold, set a different status/explanation)
matrix_benchmarking/regression.py
Outdated
self.regression_test( | ||
curr_values, | ||
lts_values | ||
) |
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.
can you save the result of this call into a variable, and use the variable here?
this is a key the important part of the function body, shouldn't be "hidden" in the middle of the object creation
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.
Sure I'll make those more clear
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.
@kpouget How do you mean: and everything in __init__.py
?
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.
in the regression
directory, you'll create some modules/files,
and what does not belong to a module, you put it in regression/__init__.py
, so that it's reachable with eg regression.RegressionStatus(
closing this PR, part of the framework has been integrated into #149 |
No description provided.