fix: align positional argument parsing with parameter positions - #667
Open
juanmicl wants to merge 2 commits into
Open
fix: align positional argument parsing with parameter positions#667juanmicl wants to merge 2 commits into
juanmicl wants to merge 2 commits into
Conversation
Member
|
Okay, that seems like it might some the problem. But would you mind adding some tests to validate the fix? |
Author
|
Added 9 tests to
Six of them fail on master and pass with the fix. Full suite passes locally (322 tests), ruff/black/mypy clean. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #666
parse_paramsmapped positional arguments to annotations using a counterthat only advanced for annotated parameters, so any unannotated
positional parameter shifted every following argument onto the wrong
annotation (values silently coerced by the wrong type, annotated
parameters never validated).
This PR decouples the positional slot index from annotation presence:
POSITIONAL_ONLY/POSITIONAL_OR_KEYWORDparameter advancesthe slot index, annotated or not;
*argsannotations now coerce all remaining positional arguments(previously only the first element was coerced);
Behavior for fully-annotated signatures is unchanged, all existing
test_params_parser.pycases pass as-is.Validation: full suite passes (
pytest -q, 313 tests),black,ruffand
mypyclean (mypy reports the same 2 pre-existing errors intaskiq/serializers/as on master).