-
Notifications
You must be signed in to change notification settings - Fork 7.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
array_change_key_case(): Throw ValueError on invalid argument #15883
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.
Thank you! Looks good. Also nice to get of the contrived test cases.
Do we need RFC for such changes? I have similar PR in #15647 |
I don't think such changes require an RFC (maybe in theory, but barely practical). Given #16641, we may consider to be more conservative, and deprecate passing anything but |
I think we should maybe put a general RFC how to deal with this. My preference is really to go through deprecations. I don't think we need to do RFC for each case but having some agreement (e.g. or possibly a single RFC) would be good. |
Btw. if we agree that deprecations are better for those case, I don't think we need RFC. |
Those should be warnings not deprecations if we are not turning them into ValueErrors. |
I would turn them into |
This should be merged after 8.4 has been cut.