-
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
Injection automation #141
base: dev
Are you sure you want to change the base?
Injection automation #141
Conversation
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.
Looks good to me -- could use top-level and function/class docstrings in the format of other parts of the code (e.g. any of the detector implementations are a good example).
Edit: I only point out some of the missing docstrings below.
import noise | ||
|
||
|
||
def select_random_classes(series): |
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.
This could use a docstring describing the function with a section for arguments and return value
@@ -0,0 +1,589 @@ | |||
import matplotlib.pyplot as plt |
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.
A top-level docstring describing what this module is for in a short line or so would be helpful
return [class_a, class_b] | ||
|
||
|
||
class InjectionTesting: |
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.
Same thing here, a docstring which can be modeled after the docstrings in any of the detectors e.g. KdqTreeBatch
|
||
return [start_row, end_row] | ||
|
||
def train_linear_model(self, x_cols=None, y_col=None, start=0, end=0.75): |
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.
Docstring for summary, arguments, output
It also looks like the GitHub checks are failing because of tests not passing / non-100% coverage and linting/security steps missing. |
No description provided.