-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Disable numeric supports type hinting - fixes issue #5767 #5875
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
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.
We should exercise this, e.g. by adding the new define here:
pybind11/.github/workflows/ci.yml
Line 217 in 1594396
-DPYBIND11_INTERNALS_VERSION=10000000 |
(please add before DPYBIND11_INTERNALS_VERSION
; I think it's best to keep that last, for max visibility)
include/pybind11/detail/common.h
Outdated
# define PYBIND11_TYPE_GUARD_TYPE_HINT "typing_extensions.TypeGuard" | ||
#endif | ||
|
||
#ifndef PYBIND11_DISABLE_NUMERIC_SUPPORTS_HINT |
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.
Naming suggestions:
PYBIND11_DISABLE_SUPPORTS_TYPE_HINTS
PYBIND11_ARG_TYPE_HINT_FLOAT
PYBIND11_ARG_TYPE_HINT_INT
(the main idea is to consistently have "type hint" in all macros)
Note sure about PYBIND11_DISABLE_NUMERIC_SUPPORTS_TYPE_HINTS
, or PYBIND11_DISABLE_ARITHMETIC_SUPPORTS_TYPE_HINTS
, or similar. Do we want "numeric" or "arithmetic" in name?
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 for the suggestion, I looked at the other type hint macros and they all end in TYPE_HINT, and this seemed like a nice pattern.
PYBIND11_DISABLE_SUPPORTS_TYPE_HINTS
PYBIND11_FLOAT_ARGUMENT_TYPE_HINT
PYBIND11_INT_ARGUMENT_TYPE_HINT
I will test my changes, make the ci.yml change and submit again.
@gentlegiantJGC @timohl @InvincibleRMC for visibility |
I don't think the testing in the CI works as expected. I tested locally with this diff: diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h
index 0ab615b3..3275693e 100644
--- a/include/pybind11/detail/common.h
+++ b/include/pybind11/detail/common.h
@@ -298,6 +298,7 @@
# define PYBIND11_TYPE_GUARD_TYPE_HINT "typing_extensions.TypeGuard"
#endif
+#define PYBIND11_DISABLE_SUPPORTS_TYPE_HINTS
#ifndef PYBIND11_DISABLE_SUPPORTS_TYPE_HINTS
# define PYBIND11_FLOAT_ARGUMENT_TYPE_HINT "typing.SupportsFloat"
# define PYBIND11_INT_ARGUMENT_TYPE_HINT "typing.SupportsInt" We need to adjust a bunch of tests:
|
Could you create a new test case file for this and only define the macro in that file? |
I will try to make a single test that uses this macro instead of the other change - this looks straightforward, but I have never used the pybind11 testing system so we will see. |
Description
Allow suppression of the SupportsInt and SupportsFloat type hints:
See #5767
Suggested changelog entry:
Allow suppression of the SupportsInt and SupportsFloat type hints by defining PYBIND11_DISABLE_NUMERIC_SUPPORTS_HINT