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

Support decorated providers (again) #128

Merged
merged 5 commits into from
Feb 16, 2024
Merged

Support decorated providers (again) #128

merged 5 commits into from
Feb 16, 2024

Conversation

jl-wynen
Copy link
Member

Fixes #127

@jl-wynen jl-wynen force-pushed the decorated-providers branch from 41693d0 to 3b567bf Compare February 16, 2024 12:30
@YooSunYoung
Copy link
Member

I added more tests and opened a PR into this branch: #132

@jl-wynen
Copy link
Member Author

#132 does not look like it would block this PR because it doesn't test anything new.

@YooSunYoung
Copy link
Member

No it won't... just wanted to target to this branch cuz it needed this update.

@jl-wynen jl-wynen merged commit b8c19db into main Feb 16, 2024
5 checks passed
@jl-wynen jl-wynen deleted the decorated-providers branch February 16, 2024 13:59
SimonHeybrock
SimonHeybrock reviewed Feb 19, 2024
View reviewed changes
Copy link
Member

@SimonHeybrock SimonHeybrock left a comment

Choose a reason for hiding this comment

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

I suppose we should make a bugfix release for this?

docs/recipes/applying-decorators.ipynb
"P = ParamSpec('P')\n",
"R = TypeVar('R')\n",
"\n",
"def deco(f: Callable[P R]) -> Callable[P, R]:\n",
Copy link
Member

Choose a reason for hiding this comment

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

Is there a comma missing here?

Copy link
Member Author

Choose a reason for hiding this comment

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

See #134

@jl-wynen
Copy link
Member Author

I suppose we should make a bugfix release for this?

Yes

jl-wynen added a commit that referenced this pull request Feb 19, 2024
@jl-wynen
Merge pull request #134 from scipp/decorated-providers
3c8eb3a 
Fix typo introduced in #128
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimonHeybrock SimonHeybrock SimonHeybrock left review comments

@YooSunYoung YooSunYoung YooSunYoung approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support decorated providers
3 participants
@jl-wynen @YooSunYoung @SimonHeybrock