-
Notifications
You must be signed in to change notification settings - Fork 917
Fill PyTypeInfo::TYPE_HINT for built-in types #5619
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
Conversation
|
Not sure if it's fine to change the values of |
85eedf0 to
9fbb660
Compare
9fbb660 to
a91b0df
Compare
davidhewitt
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, I am happy with the changes this adds, just one tidy up I think we can do with the macro implementation before we merge.
src/types/mod.rs
Outdated
| unsafe impl<$($generics,)*> $crate::type_object::PyTypeInfo for $name { | ||
| const NAME: &'static str = stringify!($name); | ||
| const MODULE: ::std::option::Option<&'static str> = $module; | ||
|
|
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.
Idea: rather than duplicating the whole macro, what we might be able to do is have $crate::pyobject_type_info_type_hint!($type_hint_module, $type_hint_expr) macro which we call here, and that macro can be the one with two defintitions. A noop one without the feature enabled, and one which expands to const TYPE_HINT = ... with the feature enabled.
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.
Great idea! Thanks! Done d9a3e18
davidhewitt
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.
Perfect, thanks!
Makes internal macros always require a module and a name These values are then used to fill PyTypeInfo::MODULE and PyTypeInfo::TYPE_HINT (PyTypeInfo::NAME stays unchanged) This changes some values of PyTypeInfo::MODULE
Head branch was pushed to by a user without write access
|
Rebase done |
d9a3e18 to
39c7c60
Compare
|
CI seems to have failed because of an internal error in the runner system |
Makes internal macros always require a module and a name (using
builtinsfor Python builtins)These values are then used to fill
PyTypeInfo::MODULEandPyTypeInfo::TYPE_HINT(PyTypeInfo::NAMEstays unchanged)This changes some values of
PyTypeInfo::MODULE