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

allow casting optional type to numba concrete types #9161

Closed
wants to merge 1 commit into from

Conversation

dlee992
Copy link
Contributor

@dlee992 dlee992 commented Aug 24, 2023

You can see the added test case.
Without this patch, the test will fail. With this patch, we allow this kind of casting from optional to concrete_type, e.g., types.int64(optional_instance).

@dlee992
Copy link
Contributor Author

dlee992 commented Aug 25, 2023

I think this's a simple patch, tests are all passed. But how to fix the towncrier? @esc

@esc esc added the skip_release_notes Skip towncrier requirement label Aug 25, 2023
@esc esc closed this Aug 25, 2023
@esc esc reopened this Aug 25, 2023
@dlee992
Copy link
Contributor Author

dlee992 commented Aug 25, 2023

Hi, @sklam . Perhaps this PR can go into 0.58.0rc2 or 0.58.0? I think you can give this a quick review, only change one line source code and add a test for it.

@sklam sklam self-requested a review August 29, 2023 14:25
elif isinstance(val, (types.Number,
types.Boolean,
types.IntEnumMember,
types.Optional)):
Copy link
Member

Choose a reason for hiding this comment

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

Does this permit something that is optional but not like a number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In current mainline, we have this, so I think we allow casting optional to any type in the lowering level?

@lower_cast(types.Optional, types.Any)

Copy link
Contributor Author

@dlee992 dlee992 Aug 29, 2023

Choose a reason for hiding this comment

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

This PR will let typeinfer also allow casting optional to any type (?) in the typing level, I think..

@sklam
Copy link
Member

sklam commented Aug 29, 2023

Perhaps this PR can go into 0.58.0rc2 or 0.58.0?

RC2 will be limited to regressions or critical security bugs. This patch will go towards 0.59.

@esc esc added this to the 0.59.0-rc1 milestone Sep 13, 2023
@esc
Copy link
Member

esc commented Sep 13, 2023

Perhaps this PR can go into 0.58.0rc2 or 0.58.0?

RC2 will be limited to regressions or critical security bugs. This patch will go towards 0.59.

Added to the milestone for 0.59

@dlee992 dlee992 changed the title allow casting optional type to types.int64 allow casting optional type to numba concrete types Sep 21, 2023
@sklam
Copy link
Member

sklam commented Oct 17, 2023

I made an alternative PR for this at #9248
Closing.

@sklam sklam closed this Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants