-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
fix: better type to MutableMapping #12202
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: yihong0618 <[email protected]>
Signed-off-by: yihong0618 <[email protected]>
Thank you for the contribution! Although MutableMapping can offer more flexible type annotations compared to dict, I believe this doesn't quite align with our expectations: we want all dictionaries passed as parameters to be immutable. |
so keep the old way dict(input) is ok? If so I will close it |
It's okay to change |
copy that will change it |
Signed-off-by: yihong0618 <[email protected]>
@@ -31,7 +31,6 @@ def fetch( | |||
:return: the filled inputs | |||
""" | |||
results: dict[str, Any] = {} | |||
inputs = dict(inputs) |
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.
I'm sorry I didn't explain my intention clearly. In this method, we indicate to the caller that the inputs won't be directly modified by marking the parameters as a Mapping type. Then, on line 34, we make a copy of the inputs, and finally, we provide the modified inputs as a return value for the user.
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.
sorry for forget it will change these days.
Summary
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Tip
Close issue syntax:
Fixes #<issue number>
orResolves #<issue number>
, see documentation for more details.Screenshots
Checklist
Important
Please review the checklist below before submitting your pull request.
dev/reformat
(backend) andcd web && npx lint-staged
(frontend) to appease the lint gods