Message286647
Recently, I saw complains that I push changes too fast without reviews. This issue is a good example. So let me elaborate myself a little bit on these changes.
I'm not really proud of the "Rename _PyArg_ParseStack to _PyArg_ParseStackAndKeywords" change. It should be called _PyArg_ParseStackAndKeywords() from the beginning, but when I designed FASTCALL, my plan was to only check arguments (positional and keyword arguments) in the callee, and remove checks in the caller. I expected that all functions will get positional + keyword arguments.
I didn't know that not accepting keywords but only positional arguments like in str.replace(), was a deliberate language design choice.
So yes, for functions accepting only positional arguments, _PyArg_ParseStack() + _PyArg_NoStackKeywords() makes sense.
IMHO the implementation of the new _PyArg_ParseStack() and _PyArg_NoStackKeywords() functions was simple enough to avoid a review, they reuse existing code with minimum changes.
In general for FASTCALL, I only add private functions. I consider that it's ok to push code quickly, since the private property allows us to change these functions anytime.
Again, I consider that the "Argument Clinic: Use METH_FASTCALL for positionals" change is simple enough to avoid a review, I don't think that they are many ways to use FASTCALL for functions using positional-only parameters. I mean, I only see a single way to implement it. Again, if someone sees a new way to handle such parameters, we can change it anytime, Argument Clinic and FASTCALL are stil private.
More generally, FASTCALL requires a lot of tiny changes everywhere.
GitHub will allow to get reviews on long patch series. It will be easier to handle large changes. |
|
Date |
User |
Action |
Args |
2017-02-01 16:36:30 | vstinner | set | recipients:
+ vstinner, larry, methane, python-dev, serhiy.storchaka, yselivanov |
2017-02-01 16:36:30 | vstinner | set | messageid: <1485966990.73.0.644658837626.issue29286@psf.upfronthosting.co.za> |
2017-02-01 16:36:30 | vstinner | link | issue29286 messages |
2017-02-01 16:36:30 | vstinner | create | |
|