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

Make static member functions, added with def_static, staticmethod descriptor instances #1732

Merged
merged 1 commit into from
Jun 11, 2019

Conversation

YannickJadoul
Copy link
Collaborator

This PR makes sure Sphinx sees and documents static member functions as such (i.e., with an extra "static" annotation, cfr. the Python docs).

This also corresponds directly to what @staticmethod does in Python and how Boost.Python does this. One caveat, just like in pure Python, is that the staticmethod wrapper disappears when accessing the attribute, because of the descriptor protocol implementation in staticmethod.

If this is considered useful, we could also still add @classmethod?

@wjakob
Copy link
Member

wjakob commented Jun 10, 2019

Hi @YannickJadoul,

This generally looks reasonable, but I'm wondering if this is a breaking change? (i.e. one that will require a major version bump of pybind11)

AFAIK making something a static method will change the associated calling conventions, correct?

Best,
Wenzel

@YannickJadoul
Copy link
Collaborator Author

YannickJadoul commented Jun 10, 2019

@wjakob Good point, I hadn't really considered that.

However, calling conventions will not be broken. Even the object returned by Class.some_staticmethod will stay the same. The only thing this will change is the type of SomeClass.__dict__["some_staticmethod"] (which is apparently what Sphinx is looking at to add the extra static to the API documentation).

This corresponds to the effect @staticmethod has in pure Python. Additionally, since the C-implemented method types in pybind11 are already different from the types of the method you'd get in pure Python, so the types of pybind11/CPython extensions methods are already inherently duck-typed and I would think this is safe.

I've just written the following example to compare the effects of the change:

#include <pybind11/pybind11.h>

namespace py = pybind11;

class DeepThought {
public:
	static bool isAnswer(int x) { return x == 42; }
};

PYBIND11_MODULE(example, m) {
	py::class_<DeepThought>(m, "DeepThought")
		.def(py::init<>())
		.def_static("is_answer", &DeepThought::isAnswer)
		.def_static("is_answer_staticmethod", &DeepThought::isAnswer);

	m.attr("DeepThought").attr("is_answer_staticmethod") = PyStaticMethod_New(m.attr("DeepThought").attr("is_answer_staticmethod").ptr());
}
Python 3.7.1 (default, Oct 22 2018, 11:21:55) 
[GCC 8.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import example
>>> example.DeepThought.is_answer(41)
False
>>> example.DeepThought.is_answer_staticmethod(42)
True
>>> example.DeepThought().is_answer(41)
False
>>> example.DeepThought().is_answer_staticmethod(42)
True
>>> example.DeepThought.is_answer
<built-in method is_answer of PyCapsule object at 0x7fc6009c02d0>
>>> example.DeepThought.is_answer_staticmethod
<built-in method is_answer_staticmethod of PyCapsule object at 0x7fc6009c02a0>
>>> example.DeepThought.__dict__["is_answer"]
<built-in method is_answer of PyCapsule object at 0x7fc6009c02d0>
>>> example.DeepThought.__dict__["is_answer_staticmethod"]
<staticmethod object at 0x7fc5fed3e1d0>

(Also tested in 2.7, and it shows the same results)

@wjakob
Copy link
Member

wjakob commented Jun 11, 2019

Ok, this looks all good to me then! — Thanks!

@wjakob wjakob merged commit d23c821 into pybind:master Jun 11, 2019
@YannickJadoul YannickJadoul deleted the def_static-staticmethod branch September 15, 2020 15:30
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.

None yet

2 participants