search

pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.1k stars 2.05k forks source link

fix: enable `bind_map` with `using` declarations. #4952

Closed AntoinePrv closed 7 months ago

AntoinePrv commented 7 months ago

Description

Enable using py::bind_map with map-like things that privately inherit from map + using declarations.

Suggested changelog entry:

- Fix ``bind_map`` with ``using`` declarations.
rwgk commented 7 months ago

Could you please add a test that does not work without this PR? — That's the only way to ensure that there are no regressions long-term.

See tests/test_stl_binders.cpp and maybe tests/test_stl_binders.py

AntoinePrv commented 7 months ago

LGTM but could you please also add minimal tests in test_stl_bind.py? Just enough to be sure the new py::bind_vector and py::bind_map are not accidentally lost in the future.

IMHO this is not necessary. This was purely a compilation fix.

rwgk commented 7 months ago

LGTM but could you please also add minimal tests in test_stl_bind.py? Just enough to be sure the new py::bind_vector and py::bind_map are not accidentally lost in the future.

IMHO this is not necessary. This was purely a compilation fix.

That's what I understood.

But there is a chance that the two new lines are accidentally removed when refactoring.

Adding minimal tests (maybe 2 x 2 lines) is very easy and makes it much less likely that something gets lost accidentally.

AntoinePrv commented 7 months ago

I have added a Python test. Thanks for taking the time.