-
Notifications
You must be signed in to change notification settings - Fork 62
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
ENH: Remove unused default process count parameter #530
ENH: Remove unused default process count parameter #530
Conversation
943f03b
to
e35494a
Compare
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 we also make this change?
e35494a
to
8fa6548
Compare
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.
One last comment. Otherwise, all looks good.
Remove unused default process count parameter from the process count validation method. Take advantage of the commit to: - Change the "less than or equal to" 0 condition on the value to be returned to a "strictly less than": if a value of 0 is provided in the arguments, then the returned value corresponds to the available core count, and thus the value will never be less than 1. - Improve the method documentation: state clearly the range of valid values and document the method behavior if the process count is not contained in the arguments. - Fix the returned value docstring. - Fix the logged process count in script.
8fa6548
to
e733d7d
Compare
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.
Cool, I like the improved docstring!
Remove unused default process count parameter from the process count
validation method.
Take advantage of the commit to:
the arguments.