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

Add register_hooks utility #247

Closed
wants to merge 23 commits into from
Closed

Add register_hooks utility #247

wants to merge 23 commits into from

Conversation

RimRihana
Copy link
Contributor

@RimRihana RimRihana commented May 22, 2024 •
edited
Loading

This PR implements the register_hook function:

  • It wraps the target method by adding the hook to be registered to the original target method
  • It also raises a TypeError in case the signatures accept different arguments

This PR also add a simple example that shows how to use the register_hook function.

@RimRihana RimRihana added the new feature New functionality label May 22, 2024
@RimRihana RimRihana self-assigned this May 22, 2024
AVHopp
AVHopp reviewed May 22, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@RimRihana RimRihana force-pushed the dev_callables branch 3 times, most recently from a4611fc to 15b475f Compare May 29, 2024 14:09
@RimRihana RimRihana marked this pull request as draft May 29, 2024 14:11
@RimRihana RimRihana marked this pull request as ready for review May 29, 2024 14:15
@Scienfitz Scienfitz marked this pull request as draft June 3, 2024 09:03
@Scienfitz
Copy link
Collaborator

@RimRihana please publish this when you and Adrian are happy for this to be reviewed

@RimRihana RimRihana force-pushed the dev_callables branch 2 times, most recently from 7e80489 to 3cdf0da Compare June 5, 2024 09:26
@RimRihana RimRihana marked this pull request as ready for review June 5, 2024 09:27
Scienfitz
Scienfitz reviewed Jun 5, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
examples/Basics/callable.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Show resolved Hide resolved
Scienfitz
Scienfitz approved these changes Jun 11, 2024
View reviewed changes
baybe/utils/basic.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
baybe/utils/basic.py Outdated Show resolved Hide resolved
examples/Custom_Hooks/callable.py Outdated Show resolved Hide resolved
examples/Custom_Hooks/callable.py Outdated Show resolved Hide resolved
examples/Custom_Hooks/callable.py Outdated Show resolved Hide resolved
examples/Custom_Hooks/callable.py Outdated Show resolved Hide resolved
examples/Custom_Hooks/callable.py Outdated Show resolved Hide resolved
@RimRihana RimRihana force-pushed the dev_callables branch 2 times, most recently from 7f88bab to 46219bc Compare June 11, 2024 12:50
Scienfitz
Scienfitz reviewed Jun 12, 2024
View reviewed changes
examples/Custom_Hooks/callable.py Outdated Show resolved Hide resolved
examples/Custom_Hooks/callable.py Outdated Show resolved Hide resolved
@AdrianSosic AdrianSosic changed the title Add register_callable() Add register_hooks utility Jun 12, 2024
RimRihana
RimRihana commented Jun 13, 2024
View reviewed changes
CHANGELOG.md Show resolved Hide resolved
examples/Custom_Hooks/basics.py Outdated Show resolved Hide resolved
examples/Custom_Hooks/basics.py Outdated Show resolved Hide resolved
tests/test_utils.py Show resolved Hide resolved
RimRihana and others added 23 commits June 16, 2024 19:25
@RimRihana @AdrianSosic
Add register_hook
0322e97
@RimRihana @AdrianSosic
Update CHANGELOG.md
97a2729
@RimRihana @AdrianSosic
Add basic callable example
633aa66
@RimRihana @AdrianSosic
Add register_hook tests
8e48e21
@AdrianSosic
Fix wording
eed3062
@AdrianSosic
Redesign example
94cc563
@AdrianSosic
Drop unnecessary tests
f38ea3d
@AdrianSosic
Refactor names in test
2284d45
@AdrianSosic
Add failing test case: missing parameters
8944aab
@AdrianSosic
Add check for same number of parameters
183073d
@AdrianSosic
Reveal failing test case: different names not annotated
38de69a
@AdrianSosic
Fix validation logic
d39978b
@AdrianSosic
Extend registration logic
748ad58 
* Differentiate between pre- and post-hooks
* Allow multiple hooks per hook category
@AdrianSosic
Add timer hook
3af714d
@AdrianSosic
Rephrase example header
48e6ab4
@AdrianSosic
Rename example file
0cfaf4b
@AdrianSosic
Update docstring of register_hooks
10ffec6
@AdrianSosic
Drop searchspace annotations
0d4b791
@AdrianSosic
Refactor argument matching logic
67efa44 
* The hook signature may now expose a subset of the target parameters
* Defaults from the target are automatically reused
@AdrianSosic
Add missing blank line
8a771f8 
Required for proper rendering
@AdrianSosic
Monkeypatch instance method instead of class function
83b0906
@AdrianSosic
Explain difference between object instances
3d1b68c
@AdrianSosic
Allow arbitrary hook annotations for unannotated target parameters
e24b49b
This was referenced Jun 19, 2024
Closed
Merged
AVHopp
AVHopp approved these changes Jun 20, 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.

I am sufficiently happy with this PR to accept it.

baybe/utils/basic.py Show resolved Hide resolved
baybe/utils/basic.py Show resolved Hide resolved
@AdrianSosic
Copy link
Collaborator

Closed in favor of #275 and #276

@AdrianSosic AdrianSosic deleted the dev_callables branch June 20, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdrianSosic AdrianSosic AdrianSosic approved these changes

@Scienfitz Scienfitz Scienfitz approved these changes

@AVHopp AVHopp AVHopp approved these changes

Assignees

@RimRihana RimRihana

Labels
new feature New functionality
Projects
None yet
Milestone
No milestone
Development

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