Make the accessor interface more complete #425
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This also adds the `hasattr` and `getattr` functions which are needed with the new attribute behavior. The new functions behave exactly like their Python counterparts. Similarly `object` gets a `contains` method which calls `__contains__`, i.e. it's the same as the `in` keyword in Python.
`auto var = l[0]` has a strange quirk: `var` is actually an accessor and not an object, so any later assignment of `var = ...` would modify l[0] instead of `var`. This is surprising compared to the non-auto assignment `py::object var = l[0]; var = ...`. By overloading `operator=` on lvalue/rvalue, the expected behavior is restored even for `auto` variables.
|
Cool -- it's nice that some of the awkwardness of the accessor objects can be eliminated using rvalues. This looks good to me. I'll wait a bit before merging to give others a chance to comment. |
|
merged, thank you! |
|
I've only had a chance to look at it now, but this is great, makes accessor API super ergonomic. 👍 |
jagerman
added a commit
to jagerman/pybind11
that referenced
this pull request
Oct 2, 2016
PR pybind#425 removed the bool operator from attribute accessors. This was quite useful, and it was the only way in 1.8 to check for the existance of an attribute, via: if (obj.attr("foo")) { ... } which was both quite useful and most likely in use by pybind11 users. Since nothing in the PR pybind#425 dicussion or commit messages indicates that this was intentionally removed, I assume the removal was an oversight.
jagerman
added a commit
to jagerman/pybind11
that referenced
this pull request
Oct 2, 2016
PR pybind#425 removed the bool operator from attribute accessors. This was quite useful, and moreover it was the only way before pybind#425 added the `hasattr` function to check for the existence of an attribute, via: if (obj.attr("foo")) { ... } and as such is probably in use by more pybind11 users than just me. Since nothing in the PR pybind#425 discussion or commit messages indicates that this was intentionally removed, I assume the removal was an oversight.
jagerman
added a commit
to jagerman/pybind11
that referenced
this pull request
Oct 2, 2016
PR pybind#425 removed the bool operator from attribute accessors. This is likely in use by existing code as it was the only way before pybind#425 added the `hasattr` function to check for the existence of an attribute, via: if (obj.attr("foo")) { ... } This commit adds it back in for attr and item accessors, but with a deprecation warning to use `hasattr(obj, ...)` or `obj.contains(...)` instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in #265, this PR completes the accessor interface to match
object. (This does not include the more drastic changes ofobject/handlementioned there. This PR only affects accessors.)With the more complete accessors in this PR there is no need for an additional cast to
objectwhen working with attributes and items:All accessor support the full interface, so any kind of chain can be made without intermediate casts:
Breaking change
This is a breaking change for
handle::attrandhandle::operator[]. They will throwerror_aleady_setwith the original Python error message instead of returningnullptr. This may not be too bad as the previous behavior was not documented as far as I can see. It's also possible to do this without a breaking change, but it would require checking for errors in 3 places instead of 1 and reconstructing the Python's error messages from scratch.Extra
The first two commits here actually reduce the binary size of the test module by ~3%, which is nice. The last 3 commits then increase the size but that's because of the new tests.