-
Notifications
You must be signed in to change notification settings - Fork 267
Add PEP 3107/362 introspection to nb_func #1221
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
base: master
Are you sure you want to change the base?
Conversation
|
I am in general open to this change, but this implementation is not in a state that can be accepted. Instead of the messy/complex postprocessing of strings, let's directly generate information in the right format—basically, can you do the same thing but with much less code? Also: creating signature objects in Python is a slow operation, and one would in general construct them once and cache them anyways. Additional caching in nanobind is ABI-incompatible to prior versions and seems excessive. |
|
Finally, thorough testing of all cases will be needed. Take some of the existing bindings to test functions with different types, default arguments, keyword arguments, variable length positional and keyword arguments, keyword-only arguments, etc. |
|
Currently, if we want to make inspect work for static nb func, we could use monkey patch like this: import inspect
import symusic.core as core
nb_func_type = type(core.ScoreTick.__dict__['from_file'])
def _signature_from_nb_func(nb_func, sigcls):
text = getattr(nb_func, "__text_signature__", None)
if not text:
raise ValueError("no text signature")
sig = inspect._signature_fromstr(sigcls, nb_func, text, skip_bound_arg=False)
annotations = getattr(nb_func, "__annotations__", {})
params = [
param.replace(annotation=annotations.get(param.name, param.annotation))
for param in sig.parameters.values()
]
return sig.replace(parameters=params,
return_annotation=annotations.get("return", sig.return_annotation))
_orig = inspect._signature_from_callable
def _signature_from_callable(obj, **kwargs):
if isinstance(obj, nb_func_type):
sigcls = kwargs.get("sigcls", inspect.Signature)
try:
return _signature_from_nb_func(obj, sigcls)
except Exception:
pass
return _orig(obj, **kwargs)
inspect._signature_from_callable = _signature_from_callable
print(inspect.signature(core.ScoreTick.from_file))And the results would be: (path: 'str | os.PathLike', format: 'typing.Optional[str]' = None) -> 'symusic.core.ScoreTick' |
|
Additionally, I’d like to ask why you previously chose to use |
|
It was meant purely as a communication aid between stubgen and nanobind. |
This reverts commit 63ff54c.
…nd into feat/pep-introspection
Implement PEP-style introspection metadata (`__text_signature__`/`__signature__`) for nanobind functions and methods. - Update C++ implementation in `src/nb_introspect.cpp`. - Update tests in `tests/test_classes.py` and `tests/test_functions.*` to validate PEP-compliant signatures and annotations. This improves tooling compatibility and enables standard Python introspection for wrapped callables.
Do not treat absent/empty per-overload annotations as explicit 'typing.Any' during merge.\n\nCollect only concrete annotation strings across overloads and emit 'typing.Union[...]' when multiple distinct concrete annotations exist. If no concrete annotations are present for a parameter across overloads, emit 'typing.Any'.\n\nAlso relaxed annotation token collection so type tokens ('%') contribute even if ':' was not present. Updated tests to expect 'typing.Union[int, float]' for overloaded static_test. Ran focused tests locally.
|
I’ve run into some ambiguously defined behavior: for overloaded functions, how should we handle their annotations, text signature, and signature? I’m currently trying to merge the function signatures of overloads whose parameter names and orders match exactly, and for those that don’t match I just return an empty dict, None, or something similar. I’m not sure if this is a reasonable approach. |
|
And for those functions whose nb_signature is manually bound via nb::sig, should I directly parse the user-provided sig string, or should I still generate the text signature and annotations using the general logic? |
- add NB_INTROSPECT_SKIP_NB_SIG toggle and raise AttributeError for skipped/incompatible __signature__/__text_signature__ - keep merged annotation/text rendering consistent and update tests/stubs for the new semantics
…s on Python <= 3.10
Current StatusSummary
Testing
|
Pybind11 Implementation ReferenceI have checked pybind11 repo, and find that they do not support As for their first line of And also, pybind11 does not has things similar to |
- delete metadata_builder/param_state and parse descriptors inline - drop custom C-style string helpers in favor of std::string utilities - remove the global inspect cache; build handles per call instead - collapse annotation/signature merging into compact STL helpers - overall file shrinks by ~50% without changing runtime behavior
6ce494d to
26574b4
Compare
Explain why build_meta() replicates nb_func.cpp's state machine and strip the unused collect_anno flag to make the code less confusing.
Ensure PyObject_Call() failures are handled before inserting into the parameter list and bail out cleanly if PyList_SetItem itself fails.
- strip "nanobind::" in type_name_local() even when not demangling - add anonymous_signature_type + type caster and exported test function - add Python test ensuring annotations/docstrings never expose nanobind::
Summary
__annotations__and__text_signature__on eachnb_func/nb_methodso nanobind callables expose the PEP 3107/362 metadata that Python tooling expects.nb_func_getattroto return the cached metadata for those attributes, lettinginspect.signatureand autodoc treat regular nanobind bindings like builtin functions.docs/_ext/nanobind_patch_notes.mdfor downstream projects.inspect.signature()now succeeds for standard nanobind methods/functions because CPython consumes their text signatures. Static factories still raiseValueError; CPython does not treatnb_funcas a builtin descriptor, so wiring up__signature__or patchinginspectremains future work.Testing
I test it locally in symusic, here are some examples: