Mailman 3 June 2018 - Python-Dev

Re: [Python-Dev] Policy on refactoring/clean up
by Hervé "Kyle" MUTOMBO 27 Jun '18

27 Jun '18

In itself, the code clean up you have done is a good thing in the sense that you re-organized things and in my understanding, they look good now. In some of the teams I've been granted to work, there was a rule stating that whenever a dev would work on an enhancement/bugfix, he would create a separate ticket that would track all code refactoring related to his work. The strategy is quite different here and the suggestion was to have your refactoring be part of an enhancement/bugfix. The most valuable argument in favor of all the current reviewers is Guido van Rossum's comment on the effect of having git blame made harder. For some of the projects I'm currently working on, we have code cleanup teams where members are affected expressly to refactoring code. We might end up needing such things in the future but for now, it is wiser to leave the code as is and focus on fixing actual problems. -- *Q::Drone's Co-Founder and Co-CEO* *Hervé "Kyle" MUTOMBO* *Envoyé depuis le web.*

1 0

Re: [Python-Dev] Policy on refactoring/clean up
by Jeroen Demeyer 27 Jun '18

27 Jun '18

On 2018-06-27 00:02, Guido van Rossum wrote: > And TBH a desire to refactor a lot of code is often a sign of a > relatively new contributor who hasn't learned their way around the code > yet, so they tend to want to make the code follow their understanding > rather than letting their understanding follow the code. ...or it could be that the code is written the way it is only for historical reasons, instead of being purposely written that way.

3 2

Re: [Python-Dev] PEP 576
by Jeroen Demeyer 27 Jun '18

27 Jun '18

On 2018-06-26 21:43, Mark Shannon wrote: > https://github.com/markshannon/pep-576 This actually looks close to Victor Stinner's bpo-29259. But instead of storing the function pointer in the class, you're storing it in the instance. One concern that I have is that this might lead to code duplication. You require that every class implements its own specialized _FOO_FastcallKeywords() function. So you end up with _PyCFunction_FastCallKeywords(), _PyMethodDescr_FastCallKeywords(), _PyFunction_FastCallKeywords(). If I want to implement a similar class myself, I have to reinvent that same wheel again. With PEP 580, I replace all those _FOO_FastCallKeywords() functions by one PyCCall_FASTCALL() function. Admittedly, my PyCCall_FASTCALL() is more complex than each of those _FOO_FastcallKeywords() individually. But overall, I think that PEP 580 leads to simpler code. Second, you still have a performance problem for methods. You made sure that the method optimizations in the Python bytecode interpreter continue to work, but method calls from C will be slowed down. I don't know to what extent and whether it really matters, but it's something to note. Jeroen.

1 0

Policy on refactoring/clean up
by Jeroen Demeyer 27 Jun '18

27 Jun '18

Hello, On https://github.com/python/cpython/pull/7909 I encountered friction for a PR which I expected to be uncontroversial: it just moves some code without changing any functionality. So basically my question is: is there some CPython policy *against* refactoring code to make it easier to read and write? (Note that I'm not talking about pure style issues here) Background: cpython has a source file "call.c" (introduced in https://github.com/python/cpython/pull/12) but the corresponding declarations are split over several .h files. While working on PEP 580, I found this slightly confusing. I decided that it would make more sense to group all these declarations in a new file "call.h". That's what PR 7909 does. In my opinion, the resulting code is easier to read. It also defines a clear place for declarations of future functionality added to "call.c" (for example, if we add a public API for FASTCALL). Finally, I added/clarified a few comments. I expected the PR to be either ignored or accepted. However, I received a negative reaction from Inada Naoki on it. I don't mind closing the PR and keeping the status quo if there is a general agreement. However, I'm afraid that a future reviewer of PEP 580 might say "your includes are a mess" and he will be right. Jeroen.

7 6

Re: [Python-Dev] PEP 576
by Jeroen Demeyer 26 Jun '18

26 Jun '18

On 2018-06-26 21:43, Mark Shannon wrote: > https://github.com/markshannon/pep-576 Your version of PEP 576 looks very different from the "official" PEP 576 at https://www.python.org/dev/peps/pep-0576/ So can you please make a pull request to https://github.com/python/peps/pulls Also feel free to add pointers to your PEP on PEP 579. Jeroen.

1 0

PEP 576
by Mark Shannon 26 Jun '18

26 Jun '18

Hi all, Just a reminder that PEP 576 still exists as a lightweight alternative to PEP 575/580. It achieves the same goals as PEP 580 but is much smaller. https://github.com/markshannon/pep-576 Unless there is a big rush, I would like to do some experiments as to whether the new calling convention should be typedef (*callptr)(PyObject *func, PyObject *const *stack, Py_ssize_t nargs, PyObject *kwnames); or whether the increased generality of: typedef (*callptr)(PyObject *func, PyObject *const *stack, Py_ssize_t nargs, PyObject *kwnames, PyTupleObject *starargs, PyObject *kwdict); is a worthwhile enhancement. An implementation can be found here: https://github.com/python/cpython/compare/master...markshannon:pep-576-mini… Cheers, Mark.

1 0

We now have C code coverage!
by Brett Cannon 26 Jun '18

26 Jun '18

Thanks to a PR from Ammar Askar we now run Python under lcov as part of the code coverage build. And thanks to codecov.io automatically merging code coverage reports we get a complete report of our coverage (the first results of which can now be seen at https://codecov.io/gh/python/cpython). And funny enough the coverage average changed less than 1%. :)

7 11

Re: [Python-Dev] Policy on refactoring/clean up
by Jeroen Demeyer 26 Jun '18

26 Jun '18

On 2018-06-26 13:11, Ivan Pozdeev via Python-Dev wrote: > AFAICS, your PR is not a strict improvement What does "strict improvement" even mean? Many changes are not strict improvements, but still useful to have. Inada pointed me to YAGNI (https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it) but I disagree with that premise: there is a large gray zone between "completely useless" and "really needed". My PR falls in that gap of "nice to have but we can do without it". > You may suggest it as a supplemental PR to PEP 580. Or even a part of > it, but since the changes are controversial, better make the > refactorings into separate commits so they can be rolled back separately > if needed. If those refactorings are rejected now, won't they be rejected as part of PEP 580 also?

3 4

Re: [Python-Dev] Policy on refactoring/clean up
by Jeroen Demeyer 26 Jun '18

26 Jun '18

On 2018-06-26 13:54, Ivan Pozdeev via Python-Dev wrote: > This is exactly what that the YAGNI principle is about, and Inada was > right to point to it. Until you have an immediate practical need for > something, you don't really know the shape and form for it that you will > be the most comfortable with. Thus any "would be nice to have" > tinkerings are essentially a waste of time and possibly a degradation, > too: you'll very likely have to change them again when the real need > arises -- while having to live with any drawbacks in the meantime. It is important to clarify that this is exactly what I did. I *have* an implementation of PEP 580 and it's based on that PR 7909. I just think that this PR makes sense independently of whether PEP 580 will be accepted. > So, if you suggest those changes together with the PEP 580 PR That sounds like a bad idea because that would be mixing two issues in one PR. If I want to increase my chances of getting PEP 580 and its implementation accepted, I shouldn't bring in unrelated changes. To put it in a different perspective: if somebody else would make a PR to one of my projects doing a refactoring and adding new features, I would ask them to split it up.

2 1

Re: [Python-Dev] Policy on refactoring/clean up
by Jeroen Demeyer 26 Jun '18

26 Jun '18

On 2018-06-26 13:54, INADA Naoki wrote: > Real need is important than my preference. If it is needed PEP 580, I'm OK. Of course it's not needed. I never claimed that it was. I think it's *nice to have* right now and slightly more *nice to have* when changes/additions are made to call.c in the future.

1 0

Python-Dev June 2018