-
-
Notifications
You must be signed in to change notification settings - Fork 947
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
Typing for BaseConverter convert doesn't respect CONSUME_MULTIPLE_SEGMENTS #2396
Comments
... although, falcon/falcon/routing/converters.py Line 39 in 6d7a45b
I'm guessing there's no way to express that relationship in typing. If not I don't know how much appetite to e.g. make the current |
... I thought I could workaround with: Replace: class MyConvertor(BaseConverter):
CONSUME_MULTIPLE_SEGMENTS = True With: class MyConvertor(PathConverter):
... ... but falcon/falcon/routing/converters.py Line 247 in 6d7a45b
|
I think the elegant (but breaking) API for such scalar-or-vector argument is vararg. def convert(*value: str) -> Any then |
In retrospect, even typing aside, I think this is a design mistake, I just didn't think of this specific detail before. The converter should always receive a string value, if needed, pre-joint on We cannot change this now, but we should plan a breaking change for 5.0. |
You mean that the converter should pre-join them here? falcon/falcon/routing/compiled.py Lines 1094 to 1102 in 6d7a45b
so that the current path converter becomes a no-op? Maybe I guess. Overall it would probably help if we were to require subclassing here, so we could avoid using that classvar. |
Ooh, someone suggested a good workaround using Protocols here: https://discuss.python.org/t/typing-instance-based-on-classvar/69716/2 |
Correct:
falcon/falcon/routing/converters.py
Lines 49 to 52 in 6d7a45b
Wrong, should be
value: str | list[str]
:falcon/falcon/routing/converters.py
Line 48 in 6d7a45b
The text was updated successfully, but these errors were encountered: