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

feat(types): add Union and Optional to typing.h #5165

Merged
merged 14 commits into from
Jun 15, 2024

Conversation

InvincibleRMC
Copy link
Contributor

@InvincibleRMC InvincibleRMC commented Jun 13, 2024

Description

Add Union and Optional for better static typing on the python side.

Suggested changelog entry:

``Union`` and ``Optional`` were added to ``pybind11/typing.h``

@rwgk rwgk marked this pull request as draft June 13, 2024 16:58
@rwgk
Copy link
Collaborator

rwgk commented Jun 13, 2024

Looks useful.

I just converted this to a Draft PR.

Could you please work on the GitHub Actions failures? Then click Ready for review and maybe tag me.

@InvincibleRMC
Copy link
Contributor Author

Will do. It seems to be falling into the default handle_type_name rather than the special case describe in typing.h but, will keep investigating.

@rwgk
Copy link
Collaborator

rwgk commented Jun 13, 2024

Maybe you just something like this?

https://github.com/pybind/pybind11/pull/5073/files

@InvincibleRMC
Copy link
Contributor Author

InvincibleRMC commented Jun 13, 2024

The solution was to inherit from one of the types found in cast.h 900-1035. I wasn't super sure on the best type to inherit from but, from what I could tell type::type made some sense. If there is some better type to inherit from let me know.

@InvincibleRMC InvincibleRMC marked this pull request as ready for review June 13, 2024 19:14
@InvincibleRMC
Copy link
Contributor Author

@rwgk the CI is now passing.

@InvincibleRMC InvincibleRMC marked this pull request as draft June 13, 2024 21:53
@InvincibleRMC
Copy link
Contributor Author

With further testing on actual code this does not seem to currently work.

@InvincibleRMC InvincibleRMC marked this pull request as ready for review June 13, 2024 23:36
@InvincibleRMC
Copy link
Contributor Author

@rwgk With some updated testing and switching to inheriting from py::object it seems to be all working.

@InvincibleRMC InvincibleRMC marked this pull request as draft June 13, 2024 23:45
@InvincibleRMC InvincibleRMC marked this pull request as ready for review June 14, 2024 00:01
@rwgk rwgk changed the title Add type unions and optionals to typing.h Add Union and Optional to typing.h Jun 14, 2024
Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'll run this by other maintainers for a 2nd set of eyes.

@rwgk rwgk merged commit 68405a1 into pybind:master Jun 15, 2024
83 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Jun 15, 2024
@henryiii henryiii changed the title Add Union and Optional to typing.h feat: add Union and Optional to typing.h Jun 23, 2024
@henryiii henryiii changed the title feat: add Union and Optional to typing.h feat(typing): add Union and Optional to typing.h Jun 23, 2024
@henryiii henryiii changed the title feat(typing): add Union and Optional to typing.h feat(types): add Union and Optional to typing.h Jun 23, 2024
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Jun 26, 2024
@InvincibleRMC InvincibleRMC deleted the type-unions branch July 1, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants