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

cast: Qualify symbol usage in PYBIND11_TYPE_CASTER #3758

Merged
merged 2 commits into from
Feb 25, 2022
Merged

cast: Qualify symbol usage in PYBIND11_TYPE_CASTER #3758

merged 2 commits into from
Feb 25, 2022

Conversation

EricCousineau-TRI
Copy link
Collaborator

Description

Permits using macro outside of pybind11::detail

Motivated by RobotLocomotion/drake#16625 (review)

Suggested changelog entry:

``PYBIND11_TYPE_CASTER`` now uses fully qualified symbols, so it can be used outside of ``pybind11::detail``.

@EricCousineau-TRI
cast: Qualify symbol usage in PYBIND11_TYPE_CASTER
3a164c3 
Permits using macro outside of pybind11::detail
@alesgenova alesgenova mentioned this pull request Feb 24, 2022
Merged
@rwgk
Copy link
Collaborator

rwgk commented Feb 24, 2022

Quick drive-by comment/question:

  • Looks great!
  • Any chance you could add a test? (separate test_type_caster_outside_detail (or similar) would be my first choice)
    Without the test we will always be in doubt (in the best case) or break you (more likely).

@EricCousineau-TRI
Copy link
Collaborator Author

Thanks! And great point on testing - fixed an error 😅 See 1d1dbb4

rwgk
rwgk approved these changes Feb 25, 2022
View reviewed changes
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.

Awesome. I agree it fits well into tests/test_custom_type_casters.cpp

Skylion007
Skylion007 approved these changes Feb 25, 2022
View reviewed changes
Copy link
Collaborator

@Skylion007 Skylion007 left a comment

Choose a reason for hiding this comment

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

Yet another reason to avoid using macros.

@rwgk rwgk merged commit f495dfc into pybind:master Feb 25, 2022
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Feb 25, 2022
henryiii pushed a commit to henryiii/pybind11 that referenced this pull request Mar 2, 2022
@EricCousineau-TRI @henryiii
cast: Qualify symbol usage in PYBIND11_TYPE_CASTER (pybind#3758)
7d3075e 
* cast: Qualify symbol usage in PYBIND11_TYPE_CASTER

Permits using macro outside of pybind11::detail

* fixup! review
henryiii pushed a commit that referenced this pull request Mar 2, 2022
@EricCousineau-TRI @henryiii
cast: Qualify symbol usage in PYBIND11_TYPE_CASTER (#3758)
93d68dd 
* cast: Qualify symbol usage in PYBIND11_TYPE_CASTER

Permits using macro outside of pybind11::detail

* fixup! review
@rwgk rwgk mentioned this pull request Mar 11, 2022
Merged
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Mar 29, 2022
@rwgk rwgk mentioned this pull request Feb 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwgk rwgk rwgk approved these changes

@Skylion007 Skylion007 Skylion007 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.
4 participants
@EricCousineau-TRI @rwgk @Skylion007 @henryiii