Added option for different arg/return type hints to type_caster#5358
Closed
timohl wants to merge 6 commits intopybind:masterfrom
Closed
Added option for different arg/return type hints to type_caster#5358timohl wants to merge 6 commits intopybind:masterfrom
timohl wants to merge 6 commits intopybind:masterfrom
Conversation
This allows to define different python type hints for arguments and return value. This is implemented using template class `as_return_type` in order to check if `return_name` is available in the `type_caster` at compile time. If `return_name` is not available, it falls back to the previous usage of `name` for both args and return value.
The `type_caster` previously named `os.PathLike` as both argument and return type. This is inaccurate since it also takes `str` and `byte` as argument and actually returns `Path`. This commit uses the new `return_name` feature to define argument type as `Union[os.PathLike, str, bytes]` and return type as `Path`. An assert was added to the unit test for this.
11 tasks
Contributor
Author
|
The new unit test in 7d16bad shows that it does not work for nested types, e.g., Going to pull back as a draft and try to work on that. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
This PR adds the option for defining
return_namein a customtype_caster, which allows to have different python type hints for arguments and return value. The check ifreturn_nameis available ornameshould be used as fallback is implemented using the template classas_return_type.This is entirely optional and should not impact any existing
type_caster.One exception is the
type_casterforstd::filesystem::path, which was modified to support this new feature.Here,
Union[os.PathLike, str, bytes]is the argument type hint andPathis the return type hint.This is more accurate than the previous usage of
os.PathLikefor both argument and return type hint.I have updated the unit test to check for the new signature.
Suggested changelog entry:
Possible TODOs
Custom type castersdocumentationlist[Path]-> does not workhandle_type_name(maybe some recursive template magic addingas_return_typeto subtypes)np.ndarrayas arg type hint but also takes lists and tuples (should probably benumpy.typing.ArrayLike+ maybetyping.Annotatedfor shape/dtype annotationI would love to get feedback on this!
Especially on the "Possible TODO" regarding Eigen. Should I implement that here in this PR or should I open a separate PR later?
Currently I am working on adding typing stubs to Open3D (isl-org/Open3D#6917) using
pybind11-stubgen.Applying mypy/pyright on existing example code showed that a lot of type checks failed, which could be fixed by this PR.