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

Fix automatic unwrapping of optional type #9248

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

sklam
Copy link
Member

@sklam sklam commented Oct 17, 2023

Alternative to #9161.

The root problem of optional typed arguments not unpacking is due to lack of exception handling around the typer. With the fix, we don't need to specialize individual function to handle optional types.

@sklam sklam added this to the 0.59.0-rc1 milestone Oct 17, 2023
@sklam sklam changed the title allow casting optional type to numba concrete types Automatic unwrapping of optional type Oct 18, 2023
@sklam sklam changed the title Automatic unwrapping of optional type Fix automatic unwrapping of optional type Oct 18, 2023
@sklam sklam added 3 - Ready for Review Effort - short Short size effort needed labels Oct 18, 2023
@guilhermeleobas guilhermeleobas marked this pull request as ready for review October 24, 2023 14:24
Copy link
Collaborator

@guilhermeleobas guilhermeleobas left a comment

Choose a reason for hiding this comment

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

LGTM

@guilhermeleobas guilhermeleobas added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels Nov 18, 2023
@karthikkurella
Copy link

LGTM

return x

args = list(map(unpack_opt, args))
sig = typer(*args, **kws)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the kwargs need unpacking too?

@esc
Copy link
Member

esc commented Nov 29, 2023

@karthikkurella @guilhermeleobas @sklam I am moving this back to the state: "Waiting on Author" as an additional comment has been raised by @stuartarchibald

@esc esc added 4 - Waiting on author Waiting for author to respond to review and removed 5 - Ready to merge Review and testing done, is ready to merge labels Nov 29, 2023
@sklam sklam modified the milestones: 0.59.0-rc1, 0.60.0-rc1 Dec 5, 2023
Copy link

github-actions bot commented Mar 5, 2024

This pull request is marked as stale as it has had no activity in the past 3 months. Please respond to this comment if you're still interested in working on this. Many thanks!

@github-actions github-actions bot added the stale Marker label for stale issues. label Mar 5, 2024
@guilhermeleobas guilhermeleobas added 2 - In Progress and removed stale Marker label for stale issues. labels Mar 5, 2024
@gmarkall gmarkall modified the milestones: 0.60.0-rc1, 0.61.0-rc1 Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - In Progress 4 - Waiting on author Waiting for author to respond to review Effort - short Short size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants