-
Notifications
You must be signed in to change notification settings - Fork 59
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
make the Runner work with unhashable points #268
Conversation
7e96a7c
to
8a0adf3
Compare
Codecov Report
@@ Coverage Diff @@
## master #268 +/- ##
==========================================
- Coverage 79.52% 79.35% -0.18%
==========================================
Files 32 32
Lines 4425 4446 +21
Branches 815 821 +6
==========================================
+ Hits 3519 3528 +9
- Misses 779 788 +9
- Partials 127 130 +3
Continue to review full report at Codecov.
|
33800c8
to
79a804d
Compare
I've scheduled time later this morning (pacific time) to take a look at this. |
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.
Overall I agree with the approach, but I believe that we can make everything cleaner by only dealing with point IDs inside the runner itself. We only need to extract actual points at the interface between the runner and the outside world (when calling self.learner.ask
and self._submit
)
79a804d
to
62e9a41
Compare
@jbweston, thanks a lot for your review! I've implemented all suggestions you've made 🎉 |
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.
LGTM! Nice work
6ed7f85
to
c7a12a4
Compare
Description
Enable the
Runner
to with with unhashable points.Fixes #267.
Checklist
pre-commit run --all
(first install usingpip install pre-commit
)pytest
passedType of change
Check relevant option(s).