-
Notifications
You must be signed in to change notification settings - Fork 12
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
Firstpipe module #35
Firstpipe module #35
Conversation
…olumn_wise_combination
… as keys when they are not unique in the Json
… does not brake code
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.
See small changes before merging, otherwise, this looks great!
@@ -83,7 +84,143 @@ def _check_params_schema(self) -> int: | |||
return num_params_list[0] | |||
else: | |||
raise ValueError(f"Expected the same number of values for all the params under noise value, but received a discordant ammount instead.") | |||
|
|||
|
|||
def _transform_noise_dict(self): |
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.
Should be transform_noise_dict without "_" since this calls self.
def _transform_noise_dict(self): | ||
""" | ||
TODO helper fucntion section | ||
""" |
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 should provide enough documentation with examples
|
||
|
||
|
||
def _handle_parameter_selection(self, d: dict, param_index: int) -> dict: |
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 should not have "_" as it calls self in the arguments
""" | ||
works on the self.noise_arg dictionary to compute all possible combinations of parameters and nboisers in a all against all fashion. | ||
""" | ||
|
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.
Possible to write an issue about this ?
""" | ||
TODO add description | ||
""" | ||
|
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.
Possible to write an issue about this ?
bin/launch_interpret_json.py
Outdated
#print(schema.experiment, schema.interpret_params_mode, schema.column_names) | ||
|
||
|
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.
Ideally, prints should not be used and replaced by a logger if you are using those to debug (see https://docs.python.org/3/howto/logging.html )
…er or combinations of one present and not hte other
Ingnore print please. The split reading is not yet done tis is just to process the noise part of the script. Look at examples user_given.json to see an json with all possibilities of values and parameters.