Skip to content

Add anyset & frozenset, enable copying (cast) to std::set #3901

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

Merged
rwgk merged 9 commits into from
May 5, 2022
Merged

Add anyset & frozenset, enable copying (cast) to std::set #3901

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e828031
Add frozenset, and allow it cast to std::set
ecatmur Apr 24, 2022
f2db7bb
Rename set_base to any_set to match Python C API
ecatmur Apr 24, 2022
ef92aa5
PR: static_cast, anyset
ecatmur May 1, 2022
736f293
Add tests for frozenset
ecatmur May 1, 2022
faf8a51
Remove frozenset default ctor, add tests
ecatmur May 1, 2022
05b6147
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 1, 2022
26a29f4
Merge remote-tracking branch 'upstream/master' into frozenset-core
ecatmur May 1, 2022
0cf2cae
Add rationale to `pyobject_caster` default ctor
ecatmur May 2, 2022
c63ca46
Remove ineffectual protected: access control
ecatmur May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions include/pybind11/pytypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1784,24 +1784,41 @@ class kwargs : public dict {
PYBIND11_OBJECT_DEFAULT(kwargs, dict, PyDict_Check)
};

class set : public object {
class any_set : public object {
protected:

@Skylion007 Skylion007 May 4, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Okay, last concern. Why is the scope here protected? I don't see any other Pytypes doing that.

@Skylion007 Skylion007 May 4, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Specifically, I am worried it might interfere with the static "check_" method that isinstance is suppose to be using.

pybind11/include/pybind11/pytypes.h

Line 483 in e8e229f

return T::check_(obj);

@rwgk rwgk May 4, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good question, it turns out the protected: here doesn't do anything, because the PYBIND11_OBJECT macro expands to starting with public:. See clang -E output below.

(Note that there is an isinstance<anyset> in stl.h, i.e. this had to be the case somehow.)

I think it's best to not get fancy here and to simply remove the protected: line.

class anyset : public object {
protected:
    public: [[deprecated("Use reinterpret_borrow<" "anyset" ">() or reinterpret_steal<" "anyset" ">()")]] anyset(handle h, bool is_borrowed) : object(is_borrowed ? object(h, borrowed_t{}) : object(h, stolen_t{})) {} anyset(handle h, borrowed_t) : object(h, borrowed_t{}) {} anyset(handle h, stolen_t) : object(h, stolen_t{}) {} [[deprecated("Use py::isinstance<py::python_type>(obj) instead")]] bool check() const { return m_ptr != nullptr && ((_Py_IS_TYPE(((const PyObject*)(m_ptr)), &PySet_Type) || _Py_IS_TYPE(((const PyObject*)(m_ptr)), &PyFrozenSet_Type) || PyType_IsSubtype((((PyObject*)(m_ptr))->ob_type), &PySet_Type) || PyType_IsSubtype((((PyObject*)(m_ptr))->ob_type), &PyFrozenSet_Type)) != 0); } static bool check_(handle h) { return h.ptr() != nullptr && (_Py_IS_TYPE(((const PyObject*)(h.ptr())), &PySet_Type) || _Py_IS_TYPE(((const PyObject*)(h.ptr())), &PyFrozenSet_Type) || PyType_IsSubtype((((PyObject*)(h.ptr()))->ob_type), &PySet_Type) || PyType_IsSubtype((((PyObject*)(h.ptr()))->ob_type), &PyFrozenSet_Type)); } template <typename Policy_> anyset(const ::pybind11::detail::accessor<Policy_> &a) : anyset(object(a)) {} anyset(const object &o) : object(o) { if (m_ptr && !check_(m_ptr)) throw ::pybind11::type_error("Object of type '" + ::pybind11::detail::get_fully_qualified_tp_name((((PyObject*)(m_ptr))->ob_type)) + "' is not an instance of '" "anyset" "'"); } anyset(object &&o) : object(std::move(o)) { if (m_ptr && !check_(m_ptr)) throw ::pybind11::type_error("Object of type '" + ::pybind11::detail::get_fully_qualified_tp_name((((PyObject*)(m_ptr))->ob_type)) + "' is not an instance of '" "anyset" "'"); }

public:
    size_t size() const { return static_cast<size_t>(PySet_Size(m_ptr)); }
    bool empty() const { return size() == 0; }
    template <typename T>
    bool contains(T &&val) const {
        return PySet_Contains(m_ptr, detail::object_or_cast(std::forward<T>(val)).ptr()) == 1;
    }
};

@ecatmur ecatmur May 4, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, yes. Removed.

PYBIND11_OBJECT(any_set, object, PyAnySet_Check)

public:
size_t size() const { return (size_t) PySet_Size(m_ptr); }
Comment thread
ecatmur marked this conversation as resolved.
Outdated
bool empty() const { return size() == 0; }
template <typename T>
bool contains(T &&val) const {
return PySet_Contains(m_ptr, detail::object_or_cast(std::forward<T>(val)).ptr()) == 1;
}
};

class set : public any_set {
public:
PYBIND11_OBJECT_CVT(set, object, PySet_Check, PySet_New)
set() : object(PySet_New(nullptr), stolen_t{}) {
PYBIND11_OBJECT_CVT(set, any_set, PySet_Check, PySet_New)
set() : any_set(PySet_New(nullptr), stolen_t{}) {
if (!m_ptr) {
pybind11_fail("Could not allocate set object!");
Comment thread
Skylion007 marked this conversation as resolved.
}
}
size_t size() const { return (size_t) PySet_Size(m_ptr); }
bool empty() const { return size() == 0; }
template <typename T>
bool add(T &&val) /* py-non-const */ {
return PySet_Add(m_ptr, detail::object_or_cast(std::forward<T>(val)).ptr()) == 0;
}
void clear() /* py-non-const */ { PySet_Clear(m_ptr); }
template <typename T>
bool contains(T &&val) const {
return PySet_Contains(m_ptr, detail::object_or_cast(std::forward<T>(val)).ptr()) == 1;
};

class frozenset : public any_set {
Comment thread
ecatmur marked this conversation as resolved.
Outdated
public:
PYBIND11_OBJECT_CVT(frozenset, any_set, PyFrozenSet_Check, PyFrozenSet_New)
frozenset() : any_set(PyFrozenSet_New(nullptr), stolen_t{}) {
Comment thread
ecatmur marked this conversation as resolved.
Outdated
if (!m_ptr) {
pybind11_fail("Could not allocate frozenset object!");
}
}
};

Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/stl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ struct set_caster {
using key_conv = make_caster<Key>;

bool load(handle src, bool convert) {
if (!isinstance<pybind11::set>(src)) {
if (!isinstance<any_set>(src)) {
return false;
}
auto s = reinterpret_borrow<pybind11::set>(src);
auto s = reinterpret_borrow<any_set>(src);
value.clear();
for (auto entry : s) {
key_conv conv;
Expand Down
1 change: 1 addition & 0 deletions tests/test_stl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def test_set(doc):
assert s == {"key1", "key2"}
s.add("key3")
assert m.load_set(s)
assert m.load_set(frozenset(s))

assert doc(m.cast_set) == "cast_set() -> Set[str]"
assert doc(m.load_set) == "load_set(arg0: Set[str]) -> bool"
Expand Down