-
Notifications
You must be signed in to change notification settings - Fork 141
updated all __all__ lists in each module #401
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
Conversation
mihaic
left a comment
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.
brainiak/hyperparamopt/hpo.py
Outdated
|
|
||
| __all__ = [ | ||
| "get_sigma", "gmm_1d_distribution", "get_next_sample", | ||
| "fmin" |
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.
I think only fmin should be listed here. I made issue #402 about renaming the others, but we could already list only what is necessary in __all__. What do you think?
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.
I am fine with that. I did not read into detail of what other commands do.
| "ConditionSpec", | ||
| "SingleConditionSpec", | ||
| "mask_image", | ||
| "MaskedMultiSubjectData", |
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.
I was trying to ordered __all__ alphabetically.
brainiak/image.py
Outdated
| "mask_image", | ||
| "MaskedMultiSubjectData", | ||
| "multimask_images", | ||
| "mask_images" |
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.
Let's add a comma after the last element as well in multi-line lists, to minimize the changes made later on. See PEP 8:
https://www.python.org/dev/peps/pep-0008/#when-to-use-trailing-commas
brainiak/isc.py
Outdated
| "permute_two_sample_iscs", | ||
| "permutation_isc", | ||
| "timeshift_isc", | ||
| "phaseshift_isc" |
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.
@snastase, are all these supposed to be used outside BrainIAK?
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.
Okay, I fixed up all and prepended underscores in the appropriate spots for isc.py
brainiak/utils/utils.py
Outdated
| "ecdf", | ||
| "p_from_null" | ||
| "p_from_null", | ||
| "compute_p_from_null_distribution" |
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.
Could you please insert the new ones alphabetically?
some functions are removed from __all__ in hpo.py
|
See PR #408 for a fix for the current build failure. |
This PR is to fix the issue of #253