Skip to content
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

remove default parameter assignment on getcolors. Fixes #2097. #2098

Merged
merged 1 commit into from Aug 22, 2016
Merged

remove default parameter assignment on getcolors. Fixes #2097. #2098

merged 1 commit into from Aug 22, 2016

Conversation

@ghost
Copy link

@ghost ghost commented Aug 22, 2016

changed colors=range(16, 240) to colors=None.
Added if statement to set colors=range(16, 240) if parameter not provided.

changed colors=range(16, 240) to colors=None. 
Added if statement to set colors=range(16, 240) if parameter not provided.
@cdatrobot
Copy link

@cdatrobot cdatrobot commented Aug 22, 2016

Basic content checks failed!

commit 4bb261fd adds bad whitespace:
Packages/vcs/vcs/utils.py:1138: trailing whitespace.
+    

Branch-at: 4bb261f
Rejected-by: @llnlbot

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Aug 22, 2016

@embrown I liked the idea. Can you fix the style?

@cdatrobot
Copy link

@cdatrobot cdatrobot commented Aug 22, 2016

Basic content checks failed!

commit 4bb261fd adds bad whitespace:
Packages/vcs/vcs/utils.py:1138: trailing whitespace.
+    

Branch-at: 4bb261f
Rejected-by: @llnlbot

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Aug 22, 2016

@embrown is just for the doc? Makes the code look silly, like we don't know about default values.

@cdatrobot
Copy link

@cdatrobot cdatrobot commented Aug 22, 2016

Basic content checks failed!

commit 4bb261fd adds bad whitespace:
Packages/vcs/vcs/utils.py:1138: trailing whitespace.
+    

Branch-at: 4bb261f
Rejected-by: @llnlbot

@doutriaux1 doutriaux1 merged commit 1df4c87 into CDAT:master Aug 22, 2016
1 check failed
@ghost
Copy link
Author

@ghost ghost commented Aug 23, 2016

@doutriaux1 Yeah, it was primarily for the doc. When sphinx went through to generate the function prototype for this one, it spit out that full range of numbers inside the prototype, which looked horrible.
Thank you for merging!

@ghost ghost deleted the patch-3 branch Aug 23, 2016
@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Aug 23, 2016

@doutriaux1 @embrown it would have been nice to fix the bad white space before merging.

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Aug 23, 2016

yes... it will be merged next time

chaosphere2112 pushed a commit that referenced this issue Sep 1, 2016
changed colors=range(16, 240) to colors=None. 
Added if statement to set colors=range(16, 240) if parameter not provided.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants