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.

Sign up for GitHub

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

Jump to bottom

Hypothesis for objectives #197

Merged
merged 11 commits into from
Apr 11, 2024
Merged

Hypothesis for objectives #197

merged 11 commits into from
Apr 11, 2024

Conversation

AdrianSosic
Copy link
Collaborator

Adds the hypothesis strategies for our objective classes.

@AdrianSosic AdrianSosic added the tests Related to testing. label Apr 9, 2024
@AdrianSosic AdrianSosic added this to the Hypothesis milestone Apr 9, 2024
@AdrianSosic AdrianSosic self-assigned this Apr 9, 2024
@AdrianSosic
Copy link
Collaborator Author

Hi @AVHopp @Scienfitz. This is a short PR on top of the open #188 PR to add the strategies. Please wait with the review until the other one is merged.

@AdrianSosic AdrianSosic force-pushed the hypothesis/objectives branch 3 times, most recently from 8cada97 to 689dff5 Compare April 10, 2024 09:50
@AdrianSosic
Copy link
Collaborator Author

@AVHopp, @Scienfitz: Ok, now this one is also ready for review

Scienfitz
Scienfitz approved these changes Apr 10, 2024
View reviewed changes
Copy link
Collaborator

@Scienfitz Scienfitz left a comment

Choose a reason for hiding this comment

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

looks goodcci , minor comments

baybe/objectives/base.py Show resolved Hide resolved
tests/hypothesis_strategies/targets.py Outdated Show resolved Hide resolved
tests/hypothesis_strategies/objectives.py Show resolved Hide resolved
AVHopp
AVHopp reviewed Apr 11, 2024
View reviewed changes
Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

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

Looking good in general, but there are still some parts I'd like your opinion on.

baybe/objectives/desirability.py Show resolved Hide resolved
baybe/objectives/desirability.py Show resolved Hide resolved
baybe/objectives/desirability.py Show resolved Hide resolved
tests/hypothesis_strategies/objectives.py Show resolved Hide resolved
tests/hypothesis_strategies/objectives.py Show resolved Hide resolved
tests/hypothesis_strategies/parameters.py Show resolved Hide resolved
tests/hypothesis_strategies/targets.py Outdated Show resolved Hide resolved
tests/hypothesis_strategies/targets.py Outdated Show resolved Hide resolved
tests/hypothesis_strategies/utils.py Show resolved Hide resolved
tests/serialization/test_parameter_serialization.py Show resolved Hide resolved
@AdrianSosic AdrianSosic force-pushed the hypothesis/objectives branch from 689dff5 to 05248e1 Compare April 11, 2024 12:26
@AdrianSosic AdrianSosic merged commit 70f50b9 into main Apr 11, 2024
10 checks passed
@AdrianSosic AdrianSosic deleted the hypothesis/objectives branch April 11, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Scienfitz Scienfitz Scienfitz approved these changes

@AVHopp AVHopp AVHopp approved these changes

Assignees

@AdrianSosic AdrianSosic

Labels
tests Related to testing.
Projects
None yet
Milestone
Hypothesis
Development

Successfully merging this pull request may close these issues.
3 participants
@AdrianSosic @Scienfitz @AVHopp