Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.1k
[mypyc] Generate introspection signatures for compiled functions#19307
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
JukkaL left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks, this is a very useful feature! I have one question about default values, but solving this is not necessary to merge this PR.
| foropinblock.ops: | ||
| ifisinstance(op,Assign)andop.dest.name==name: | ||
| return_extract_python_literal(op.src) | ||
| return_NOT_REPRESENTABLE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do you have any ideas about how to generalize this to support more kinds of default values? I guess we could special case empty lists and dictionaries, which are somewhat common (even if you are not supposed to use them as defaults), and perhaps named constants and enum values. What about using a placeholder default value for unsupported default values, such as... (ellipsis)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Other defaults are loaded withLoadStatic ops, so we'd need to look at the module's__top_level__ IR to extract the values. That isn't too tricky to do (in fact I already havea rough patch that does that), but it could be expensive performance wise, since__top_level__ can be quite big and we'd be doing potentially many linear searches. Alternatively, we could try recording information about default arguments earlier on during IR building, though I haven't figured out a simple way to do that yet
What about using a placeholder default value for unsupported default values, such as
...(ellipsis)?
I actually tried that in a previous revision :-) I was worried that it might conflict with using an actual Ellipsis default argument at runtime. I'm also not sure what effect using a placeholder might have on the tools that consume these signatures. For example, I think stubtest might complain if a runtime signature had... as a default argument when the parameter isn't typed to actually acceptEllipsisType
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Recording information about default argument values during IR building seems like the most promising approach to me, though I'm not sure how easy this would be to implement. They could perhaps be stored inFuncDecl orFuncSignature. I agree with you that analyzing__top_level__ seems too inefficient (and also inelegant).
JukkaL left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is fine as an initial implementation. We can figure out more complex cases later.
526fec3 intopython:masterUh oh!
There was an error while loading.Please reload this page.
JukkaL commentedJul 7, 2025
@brianschubert Can you create a follow-up mypyc issue about the remaining cases, with some discussion based on the comments in this PR? |
jorenham commentedAug 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I believe that this change is why I'm seeing a bunch of new errors pop up when running stubtest onscipy-stubs using mypy master ( All errors are seem to be related to
Each parameter is reported as a separate error, and aliases are counted double, which causes 75 errors to be reported here. I must admit that I know very little about cython and the C-side of cpython, so I'm not able to see why this is happening. But the fact that theseonly occur in case of these Oh and here are the errors:https://gist.github.com/jorenham/3d41e79607bcc1f45267acc1566833d2 update: This might also be relevant infomation: >>>from scipy.stats._unuran.unuran_wrapperimport TransformedDensityRejection>>> TransformedDensityRejection.__init__.__text_signature__'($self, /, *args, **kwargs)' So it's kinda odd that stubtest says it's |
brianschubert commentedAug 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@jorenham More likely you're seeing an effect of#18259. This change only affects extension modules that are generated with mypyc, not Cython.#18259 made stubtest sensitive to signature information on C extension classes, which equally affects hand-written C extension modules and mypyc or Cython generated extension modules. |
brianschubert commentedAug 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Hmm, your issue seems to be due to stubtest looking at >>>fromscipy.stats._unuran.unuran_wrapperimportTransformedDensityRejection>>>TransformedDensityRejection.__init__.__objclass__<class'object'> For comparison, this is what happens for stdlib C extension classes: >>>importpickle>>>pickle.Pickler.__init__.__objclass__<class'_pickle.Pickler'> and for mypyc native classes: >>>importmypy.types>>>mypy.types.Instance.__init__.__objclass__<class'mypy.types.Instance'> So, it seems like there's either something funny with Cython extension classes, or a wrong assumption was made in#18259. (new issue to track this would be welcome!) |
jorenham commentedAug 25, 2025
|
Refsmypyc/mypyc#838
This PR populates
__text_signature__for compiled functions, making runtime signature introspection possible (i.e.inspect.signature(func)).While
__text_signature__is an undocumented CPython implementation detail, other extension module generators are using it in practice. For example, PyO3 and Cython both support it. I think it would be reasonable for mypyc to support it too.Some function signatures can't be represented by
__text_signature__(namely, those with complex default arguments). In those cases, no signatures are generated (same as the current behavior).