-
-
Notifications
You must be signed in to change notification settings - Fork 26k
[MRG] Added variable type check for hyperparameter.bounds #8517
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
What is the benefit of doing this? Presumably if |
Because it will return an error in the numpy developer version if the type is not checked ahead. |
Ah, got it. It looks like this PR and #8518 are similar, are you two working together? |
Ah~, nope. I just realized someone was also working on it after I committed. |
@@ -283,8 +283,9 @@ def test_set_get_params(): | |||
index = 0 | |||
params = kernel.get_params() | |||
for hyperparameter in kernel.hyperparameters: | |||
if hyperparameter.bounds == "fixed": | |||
continue | |||
if isinstance(hypterarameter.bounds, str): |
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 like this is misspelled.
Looks like travis is raising an error because there's a typo. You can write |
you can use do an "and", you don't need two "if"s |
Hm #8518 already fixed this in the meantime, sorry :-/ |
@amueller Ha, that's fine. Not a bad first try. Thank you! :-) |
Thanks for the contribution @RuiyeNi! Unfortunately, someone else got it first. :( |
My pleasure @jmschrei. I will try to make it next time :-) |
Reference Issue
Fixes #8503
What does this implement/fix? Explain your changes.
Added variable type check for hyperparameter.bounds before comparison with "fixed".
Any other comments?
Nope.