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

Check bounds_transformation is correct type #240

Merged
merged 3 commits into from
Jul 13, 2020

Conversation

alonfnt
Copy link
Contributor

@alonfnt alonfnt commented Jul 7, 2020

A quick check following the 'don't ask for permission ask for forgiveness' motto to make sure that the bounds_transformer used can be called otherwise a clear exception message is shown.

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2020

Codecov Report

Merging #240 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #240   +/-   ##
=======================================
  Coverage   98.44%   98.44%           
=======================================
  Files           7        7           
  Lines         385      387    +2     
  Branches       39       39           
=======================================
+ Hits          379      381    +2     
  Misses          3        3           
  Partials        3        3           
Impacted Files Coverage Δ
bayes_opt/bayesian_optimization.py 99.03% <ø> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb28df8...cedf5eb. Read the comment docs.

Copy link
Member

@fmfn fmfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution =)

try:
self._bounds_transformer.initialize(self._space)
except (AttributeError, TypeError):
raise AssertionError('The transformer must be instance of'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is AssertionError the right choice? Maybe TypeError instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! TypeError is a better fit. Fixed.

@fmfn fmfn merged commit 380b0d5 into bayesian-optimization:master Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants