[Python-Dev] Re: [Steering-council] Re: PEP 651, Robust Stack Overflow Handling, Rejection notice

[Mark Shannon]

On 31/01/2022 5:23 am, Gregory P. Smith wrote:

-cc: python-steering-council

On Fri, Mar 5, 2021 at 4:26 PM Guido van Rossum mailto:gu...@python.org>> wrote:

On Fri, Mar 5, 2021 at 11:11 AM Brett Cannon mailto:br...@python.org>> wrote:

Speaking for myself ...


Ditto ...

On Fri, Mar 5, 2021 at 7:04 AM Mark Shannon mailto:m...@hotpy.org>> wrote:
[...]

In some cases, the PEP would have improved the situation.

For example:
sys.setrecursionlimit(5000)
def f():
      f()

Currently, it raises a RecursionError on linux, but crashes the
interpreter on Windows.
With PEP 651 it would have raised a RecursionError on both 
platforms.

Am I missing something here?


So your example shows a user already comfortable in raising their 
recursion limit to work around needing more stack space to reach completion. 
What is stopping the user from continuing to raise the limit until they still 
reach their memory limit even with PEP 651? If you're worried about runaway 
recursion you will very likely hit that with the default stack depth already, 
so I personally don't see how a decoupled stack counter from the C stack 
specifically makes it any easier/better to detect runaway recursion. And if I 
need more recursion than the default, you're going to bump the recursion depth 
anyway, which weakens the protection in either the C or decoupled counter 
scenarios. Sure, it's currently platform-specific, but plenty of people want to 
push that limit based on their machine anyway and don't need consistency on 
platforms they will never run on, i.e. I don't see a huge benefit to being able 
to say that an algorithm consistently won't go past 5000 calls on
all platforms compared to what the C stack protection already gives us (not to 
say there's zero benefit, but it isn't massive or widespread either IMO). I personally 
just don't see many people saying, "I really want to limit my program to an exact 
call stack depth of 5000 on all platforms which is beyond the default, but anything under 
is fine and anything over -- regardless of what the system can actually handle -- is 
unacceptable".

Tack on the amount of changes required to give a cross-platform stack 
count and limit check compared to the benefit being proposed, and to me that 
pushes what the PEP is proposing into net-negative payoff.


To me, the point of that example is as a reminder that currently fiddling 
with the recursion limit can cause segfaults.

Mark's PEP proposes two, somewhat independent, changes: (1) don't consume C 
stack on pure Python-to-Python (pp2p) function calls; (2) implement fool-proof 
C stack overflow checks.

Change (2) makes it safe for users to mess with the stack overflow limit 
however they see fit. Despite (1), the limit for pp2p calls remains at 1000 so 
that users who unintentionally write some naively recursive code don't have to 
wait until they fill up all of memory before they get a traceback. (Of course 
they could also write a while-True loop that keeps adding an item to a list and 
they'd end up in the same situation. But in my experience that situation is 
less painful to deal with than accidental stack overflow, and I'd shudder at 
the thought of a traceback of a million lines.)

Given that we have (1), why is (2) still needed? Because there are ways to 
recursively call Python code that aren't pp2p calls. By a pp2p (pure 
Python-to-Python) call, I mean any direct call, e.g. a method call or a 
function call. But what about other operations that can invoke Python code? 
E.g. if we have a dict d and a class C, we could create an instance of C and 
use it to index d, e.g. d[C()]. This operation is not a p2pp call -- the 
BINARY_SUBSCR opcode calls the dict's `__getitem__` method, and that calls the 
key's `__hash__` method. Here's a silly demonstration:
```
def f(c):
     d = {}
     return d[c]

class C:
     def __hash__(self):
         return f(self)

f(C())
```
Note that the "traceback compression" implemented for simple recursive 
calls fails here -- I just ran this code and got 2000 lines of output.

The way I imagine Mark wants to implement pp2p calls means that in this 
case each recursion step *does* add several other C stack frames, and this 
would be caught by the limit implemented in (2). I see no easy way around this 
-- after all the C code involved in the recursion could be a piece of 3rd party 
C code that itself is not at fault.

So we could choose to implement only (2), robust C stack overflow checks. 
This would require a bunch of platform-specific code, and there might be 
platforms where we don't know how to implement this (I vaguely recall a UNIX 
version where main() received a growable stack but each thread only had a fixed 
64kb stack), but those would be no worse off than tod

[Python-Dev] Re: Slowly bend the C API towards the limited API to get a stable ABI for everyone

[Petr Viktorin]

On 28. 01. 22 16:04, Victor Stinner wrote:

Hi,

There is a reason why I'm bothering C extensions maintainers and
Python core developers with my incompatible C API changes since Python
3.8. Let me share my plan with you :-)


In 2009 (Python 3.2), Martin v. Löwis did an amazing job with the PEP
384 "Defining a Stable ABI" to provide a "limited C API" and a "stable
ABI" for C extensions: build an extension once, use it on multiple
Python versions. Some projects like PyQt5 and cryptograpy use it, but
it is just a drop in the PyPI ocean (353,084 projects). I'm trying to
bend the "default" C API towards this "limited C API" to make it
possible tomorrow to build *more* C extensions for the stable ABI.

My goal is that the stable ABI would be the default, and only a
minority of C extensions would opt-out because they need to access to
more functions for best performance.

The basic problem is that at the ABI level, C extensions must only
call functions, rather than getting and setting directly to structure
members. Structures changes frequently in Python (look at changes
between Python 3.2 and Python 3.11), and any minor structure change
breaks the ABI. The limited C API hides structures and only use
function calls to solve this problem.


This is not true. The limited C API does include some structs that are 
not opaque, including some fields of PyObject.
Your effort is not only bending the "regular" C API towards the limited 
API, but it's *also* bending the limited API towards a struct-less future.


This will be a better future if we get there, but getting there has its 
downsides. One downside is that making incompatible changes to the 
limited API could make it very hard to support and test the stable ABI.


For example, stable ABI extensions that do `obj->ob_type` must continue 
to work*, even if we make it impossible to do this in new extensions (by 
making PyObject opaque).
Making PyObject opaque is possible (the limited API is not stable), but 
not easy to do correctly (e.g. remember to add tests for the newly 
"unreachable" parts of the stable ABI).



(* we could also break the stable ABI, and we could even do it 
reasonably safely over a long period of time, but that's a whole 
different discussion.)




Since 2020, I'm modifying the C API, one function by one, to slowly
hide implementations (prepare the API to make strutures opaque). I
focused on the following structures:

* PyObject and PyVarObject (bpo-39573)
* PyTypeObject (bpo-40170)
* PyFrameObject (bpo-40421)
* PyThreadState (bpo-39947)

The majority of C extensions use functions and macros, they don't
access directly structure members. There are a few members which are
sometimes accessed directly which prevents making these structures
opaque. For example, some old C extensions use obj->ob_type rather
than Py_TYPE(obj). Fixing the minority of C extensiisons should benefit
to the majority which may become compatible with the stable ABI.

I am also converting macros to static inline functions to fix their
API: define parameter types, result type and avoid surprising macros
side effects ("macro pitfalls"). I wrote the PEP 670 "Convert macros
to functions in the Python C API" for these changes.


I wrote the upgrade_pythoncapi.py tool in my pythoncapi_project (*)
which modify C code to use Py_TYPE(), Py_SIZE() and Py_REFCNT() rather
than accessing directly PyObject and PyVarObject members.

(*) https://github.com/pythoncapi/pythoncapi_compat

In this tool, I also added "Borrow" variant of functions like
PyFrame_GetCode() which returns a strong reference, to replace
frame->f_code with _PyFrame_GetCodeBorrow(). In Python 3.11, you
cannot use the frame->f_code member anymore, since it has been
removed! You must call PyFrame_GetCode() (or pythoncapi_compat
_PyFrame_GetCodeBorrow() variant). >
There are also a few macros which can be used as l-values like
Py_TYPE(): "Py_TYPE(type1) = type2" must now be written
"Py_SET_TYPE(type1, type2)" to avoid setting directly the tp_type type
at the ABI level. I proposed the PEP 674 "Disallow using Py_TYPE() and
Py_SIZE() macros as l-values" to solve these issues.


Currently, many "functions" are still implemented as macros or static
inline functions, so C extensions still access structure members at
the ABI level for best Python performance. Converting these to regular
functions has an impact on performance and I would prefer to first
write a PEP giving the rationale for that. >

Today, it is not possible yet to build numpy for the stable ABI. The
gap is just too large for this big C extension. But step by step, the
C API becomes closer to the limited API, and more and more code is
ready to be built for the stable ABI.


Well, these C API changes have other advantages, like preparing Python
for further optimizations, ease Python maintenance, clarify the
seperation between the limited C API and the default C API, etc. ;-)

Victor
--
Night gathers, and now my watch begins. It shall not end until my death.
___

[Python-Dev] Re: PEP 674 – Disallow using macros as l-value (version 2)

[Petr Viktorin]

On 18. 01. 22 9:30, Victor Stinner wrote:

Hi,

I made the following changes to the PEP since the version 1 (November 30):

* Elaborate the HPy section and add a new "Relationship with the HPy
project" section
* Elaborate which projects are impacted and which changes are needed
* Remove the PyPy section

The PEP can be read online:
https://python.github.io/peps/ep-0674/

And you can find the plain text below.

Victor


PEP: 674
Title: Disallow using macros as l-value


The current PEP is named "Disallow using Py_TYPE() and Py_SIZE() macros 
as l-values", which is misleading: it proposes to change 65 macros.



Author: Victor Stinner 
Status: Draft
Type: Standards Track
Content-Type: text/x-rst
Created: 30-Nov-2021
Python-Version: 3.11

Abstract


Incompatible C API change disallowing using macros as l-value to:

* Allow evolving CPython internals;


In the current PEP, this is now  "Allow evolving CPython internals 
(change the PyObject structure);"


But that's not really true: the PyObject won't become changeable if the 
PEP is accepted, since `ob_type` is part of the stable ABI, and direct 
access to this field is compiled into existing extensions that use the 
stable ABI.


This PEP alone won't really help nogil.


* Ease the C API implementation on other Python implementation;
* Help migrating existing C extensions to the HPy API.

On the PyPI top 5000 projects, only 14 projects (0.3%) are affected by 4
macro changes. Moreover, 24 projects just have to regenerate their
Cython code to use ``Py_SET_TYPE()``.


This data was gathered in 2022.
Since this change was made in 2020 and then reverted because it broke 
too many projects (which were presumably notified of the breakage and 
had 2 years to update), the 0.3% is misleading. It is a measure of the 
current porting effort, not an estimate of the chance of a given project 
being affected.




In practice, the majority of affected projects only have to make two
changes:

* Replace ``Py_TYPE(obj) = new_type;``
   with ``Py_SET_TYPE(obj, new_type);``.
* Replace ``Py_SIZE(obj) = new_size;``
   with ``Py_SET_SIZE(obj, new_size);``.




I'll post my counterproposal, please tell me what's wrong with it (and 
ideally add it to Rejected ideas):


- document that these macros should not be used as l-values
- and that's it for CPython.

When this is done, interested people can try compiling projects with 
HPy, and send fixes to various projects. The above documentation should 
be a strong reason to accept the PRs. And finding the issue is 
relatively easy -- compile with modified CPython headers.
Meanwhile, projects that don't care about HPy (yet), or projects that 
want to keep working with minimum maintenance, can do the change on 
their own timeline.
If and when the time comes and the proposed change is actually 
beneficial to all/most users in the short term (for example it is needed 
for some major performance improvement or nogil), or we can make it 
opt-in, or it becomes one of a few things that prevent CPython from 
switching to HPy, then we can do the change. Within one release, even.
The PEP doesn't propose any deprecation period because it's impractical 
to produce compiler warnings; IMO we should handle this by having a long 
"docs-only deprecation" period, rather than not having a deprecation 
period at all.



___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/TEKRMXAKLFTWSZBFOXIK35TAKWMWQISA/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: PEP 674 – Disallow using macros as l-value (version 2)

[Victor Stinner]

On Mon, Jan 31, 2022 at 2:31 PM Petr Viktorin  wrote:
> > PEP: 674
> > Title: Disallow using macros as l-value
>
> The current PEP is named "Disallow using Py_TYPE() and Py_SIZE() macros
> as l-values", which is misleading: it proposes to change 65 macros.

Right, I made changes since I posted the "version 2" to python-dev 13
days ago. Since nobody replied to my thread (before you), I directly
submitted the latest (updated) PEP to the Steering Council:
https://github.com/python/steering-council/issues/100

I decided to put "Py_TYPE() and Py_SIZE()" in the title, since in
practice, only these two macros are causing troubles. The PEP changes
65 macros in total. In the latest PEP version, exactly 2 macros are
breaking projects: Py_TYPE and Py_SIZE.


> > * Allow evolving CPython internals;
>
> In the current PEP, this is now  "Allow evolving CPython internals
> (change the PyObject structure);"
>
> But that's not really true: the PyObject won't become changeable if the
> PEP is accepted, since `ob_type` is part of the stable ABI, and direct
> access to this field is compiled into existing extensions that use the
> stable ABI.

When I created https://bugs.python.org/issue39573, I chose the title
"Make PyObject an opaque structure in the limited C API". Two years
later, I changed the title to "[C API] Avoid accessing PyObject and
PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(),
disallow Py_TYPE(obj)=type" since I understood that this project
requires multiple incremental steps (and maybe changes should be done
in multiple Python versions).

You're right that the PEP 674 alone is not enough to fully make the
structure opaque. The next interesting problem is that
PyType_FromSpec() and PyType_Spec API (which are part of the stable
ABI!) use indirectly sizeof(PyObject) in the PyType_Spec.basicsize
member.

One option to make the PyObject structure opaque would be to modify
the PyObject structure to make it empty, and move its members into a
new private _PyObject structure. This _PyObject structure would be
allocated before the PyObject* pointer, same idea as the current
PyGC_Head header which is also allocated before the PyObject* pointer.

The main blocker issue is that it breaks the stable ABI.

How should the PEP abstract be rephrased to be more accurate?


> > On the PyPI top 5000 projects, only 14 projects (0.3%) are affected by 4
> > macro changes. Moreover, 24 projects just have to regenerate their
> > Cython code to use ``Py_SET_TYPE()``.
>
> This data was gathered in 2022.
> Since this change was made in 2020 and then reverted because it broke
> too many projects (which were presumably notified of the breakage and
> had 2 years to update), the 0.3% is misleading. It is a measure of the
> current porting effort, not an estimate of the chance of a given project
> being affected.

I tried to provide all data that I gathered. The following section
list 14 projects that had to be fixed:
https://www.python.org/dev/peps/pep-0674/#projects-released-with-a-fix

I didn't check if they are part of the current top 5000 PyPI project or not.

How should the abstract be rephrased to mention these projects? Does
someone know how to check if these projects are part of the top 5000
PyPI projects?

Victor
-- 
Night gathers, and now my watch begins. It shall not end until my death.
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/BVBXIAMEH3UMDN5NWV4GLRIZ5L4RRI5U/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Slowly bend the C API towards the limited API to get a stable ABI for everyone

[Victor Stinner]

On Mon, Jan 31, 2022 at 1:48 PM Petr Viktorin  wrote:
> (* we could also break the stable ABI, and we could even do it
> reasonably safely over a long period of time, but that's a whole
> different discussion.)

IMO the stable ABI must be change in the long term, it still leaks too
many implementation details. But right now, I didn't gather enough
data about the problematic APIs and what must be changed exactly. I
would prefer to only do once the work will be really blocked and there
would be no other choice.

Right now, I'm focused on fixing the *API*. It doesn't require to
break the stable ABI.

If we change the stable ABI, I would prefer to fix multiple issues at
once. Examples:

* No longer return borrowed references (ex: PyDict_GetItem is part of
the stable ABI) and no longer steal references (ex:
PyModule_AddObject)

* Disallow getting direct access into an object data without a
function to "release" the data. For example, PyBytes_AsString() gives
a direct access into the string, but Python doesn't know when the C
extension is done with it, and when it's safe to delete the object.
Such API prevents to move Python objects in memory (implement a moving
garbage collector in Python).

* Disallow dereferencing a PyObject* pointer: most structures must be
opaque. It indirectly means that accessing directly structure members
must also be disallowed. PEP 670 and PEP 674 are partially fixing the
issues.

Victor
-- 
Night gathers, and now my watch begins. It shall not end until my death.
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/AFQVC6V4EXOOWV7LK7BHIIX2WPV5H2WX/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: PEP 674 – Disallow using macros as l-value (version 2)

[Petr Viktorin]

On 31. 01. 22 15:30, Victor Stinner wrote:

On Mon, Jan 31, 2022 at 2:31 PM Petr Viktorin  wrote:

PEP: 674
Title: Disallow using macros as l-value


The current PEP is named "Disallow using Py_TYPE() and Py_SIZE() macros
as l-values", which is misleading: it proposes to change 65 macros.


Right, I made changes since I posted the "version 2" to python-dev 13
days ago. Since nobody replied to my thread (before you), I directly
submitted the latest (updated) PEP to the Steering Council:
https://github.com/python/steering-council/issues/100

I decided to put "Py_TYPE() and Py_SIZE()" in the title, since in
practice, only these two macros are causing troubles. The PEP changes
65 macros in total. In the latest PEP version, exactly 2 macros are
breaking projects: Py_TYPE and Py_SIZE.


This is a reasonable assumption if you think that backwards-incompatible 
changes are OK to make if they don't affect "projects". (Presumably, 
projects that you know about.)
If I don't agree wit that, I can't rely on the title/abstract to decide 
how much time I'll need to spend on the PEP :(




* Allow evolving CPython internals;


In the current PEP, this is now  "Allow evolving CPython internals
(change the PyObject structure);"

But that's not really true: the PyObject won't become changeable if the
PEP is accepted, since `ob_type` is part of the stable ABI, and direct
access to this field is compiled into existing extensions that use the
stable ABI.


When I created https://bugs.python.org/issue39573, I chose the title
"Make PyObject an opaque structure in the limited C API". Two years
later, I changed the title to "[C API] Avoid accessing PyObject and
PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(),
disallow Py_TYPE(obj)=type" since I understood that this project
requires multiple incremental steps (and maybe changes should be done
in multiple Python versions).

You're right that the PEP 674 alone is not enough to fully make the
structure opaque. The next interesting problem is that
PyType_FromSpec() and PyType_Spec API (which are part of the stable
ABI!) use indirectly sizeof(PyObject) in the PyType_Spec.basicsize
member.

One option to make the PyObject structure opaque would be to modify
the PyObject structure to make it empty, and move its members into a
new private _PyObject structure. This _PyObject structure would be
allocated before the PyObject* pointer, same idea as the current
PyGC_Head header which is also allocated before the PyObject* pointer.

The main blocker issue is that it breaks the stable ABI.

How should the PEP abstract be rephrased to be more accurate?


Remove the point from the Abstract?


On the PyPI top 5000 projects, only 14 projects (0.3%) are affected by 4
macro changes. Moreover, 24 projects just have to regenerate their
Cython code to use ``Py_SET_TYPE()``.


This data was gathered in 2022.
Since this change was made in 2020 and then reverted because it broke
too many projects (which were presumably notified of the breakage and
had 2 years to update), the 0.3% is misleading. It is a measure of the
current porting effort, not an estimate of the chance of a given project
being affected.


I tried to provide all data that I gathered. The following section
list 14 projects that had to be fixed:
https://www.python.org/dev/peps/pep-0674/#projects-released-with-a-fix

I didn't check if they are part of the current top 5000 PyPI project or not.

How should the abstract be rephrased to mention these projects? Does
someone know how to check if these projects are part of the top 5000
PyPI projects?


I think you should make it clear that the percentage was taken after 
fixing many of the projects.
That change will probably make the numbers look rather meaningless. So 
either explain why they're meaningful, or remove the paragraph.


(Somewhat unrelated: you could also put the motivation in a Motivation 
section, rather than the Abstract. If that leaves the abstract with just 
one sentence, it's good -- provided the sentence is still a good summary 
of the proposed changes.)


___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/LDX6O76V7M3BWZKWXQHPJ26GODS6Z5UE/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Slowly bend the C API towards the limited API to get a stable ABI for everyone

[Petr Viktorin]

On 31. 01. 22 15:40, Victor Stinner wrote:

On Mon, Jan 31, 2022 at 1:48 PM Petr Viktorin  wrote:

(* we could also break the stable ABI, and we could even do it
reasonably safely over a long period of time, but that's a whole
different discussion.)


IMO the stable ABI must be change in the long term, it still leaks too
many implementation details. But right now, I didn't gather enough
data about the problematic APIs and what must be changed exactly. I
would prefer to only do once the work will be really blocked and there
would be no other choice.

Right now, I'm focused on fixing the *API*. It doesn't require to
break the stable ABI.

If we change the stable ABI, I would prefer to fix multiple issues at
once. Examples:

* No longer return borrowed references (ex: PyDict_GetItem is part of
the stable ABI) and no longer steal references (ex:
PyModule_AddObject)

* Disallow getting direct access into an object data without a
function to "release" the data. For example, PyBytes_AsString() gives
a direct access into the string, but Python doesn't know when the C
extension is done with it, and when it's safe to delete the object.
Such API prevents to move Python objects in memory (implement a moving
garbage collector in Python).

* Disallow dereferencing a PyObject* pointer: most structures must be
opaque. It indirectly means that accessing directly structure members
must also be disallowed. PEP 670 and PEP 674 are partially fixing the
issues.


All of these can be changed in the API. Not easily -- I mentioned the 
problem with testing the ABI, but there might be others -- but fixing 
these in the API first is probably the way to go.


The ABI can then be changed to align with the current API.
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/5A6A5QQ2HWVP7BHY36SFK3TP7A4MMX6I/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Slowly bend the C API towards the limited API to get a stable ABI for everyone

[Victor Stinner]

On Mon, Jan 31, 2022 at 4:03 PM Petr Viktorin  wrote:
> > If we change the stable ABI, I would prefer to fix multiple issues at
> > once. Examples:
> >
> > * No longer return borrowed references (ex: PyDict_GetItem is part of
> > the stable ABI) and no longer steal references (ex:
> > PyModule_AddObject)
> >
> > * Disallow getting direct access into an object data without a
> > function to "release" the data. For example, PyBytes_AsString() gives
> > a direct access into the string, but Python doesn't know when the C
> > extension is done with it, and when it's safe to delete the object.
> > Such API prevents to move Python objects in memory (implement a moving
> > garbage collector in Python).
> >
> > * Disallow dereferencing a PyObject* pointer: most structures must be
> > opaque. It indirectly means that accessing directly structure members
> > must also be disallowed. PEP 670 and PEP 674 are partially fixing the
> > issues.
>
> (...) fixing these in the API first is probably the way to go.

That's what I already did in the past and what I plan to do in the future.

Victor
-- 
Night gathers, and now my watch begins. It shall not end until my death.
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/JYZFBZPHBYNOR7RZXLFFEHL7WZGNY5EA/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Ken Jin]

@Lrupert
> - Add coverage to X (tens of them, as separate PRs)

I think I know the PRs you're referring to. For the record, some of the PRs 
tested hairy code paths in the module I maintain. I would have less confidence 
backporting bugfixes touching that code if we didn't have those tests (the code 
paths predate me maintaining the module).

Sure, the PRs have overwhelmed me at times. However, I'd rather be overwhelmed 
with test coverage PRs now than with complaints I broke somebody's code on 
release day :).


Inada Naoki wrote:
> Some people may do "approval without review" to make their "Profile"
> page richer, because GitHub counts it as a contribution.

I noticed the same person you mentioned approving PRs right after I send an 
approval. It's very confusing for me.
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/ZXFZD3CLUSJ533QXC73RQQG4EHYJPV6R/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Brian Curtin]

On Sun, Jan 30, 2022 at 8:42 AM Mats Wichmann  wrote:

> On 1/30/22 04:45, Inada Naoki wrote:
> > On Sun, Jan 30, 2022 at 7:37 PM Irit Katriel 
> wrote:
>
> > Some people may do "approval without review" to make their "Profile"
> > page richer, because GitHub counts it as a contribution.
> > Creating spam issues or pull requests can be reported as spam very
> > easily. But "approve without review" is hard to be reported as spam.
> > So approving random issue is the most easy way to earn contributions
> > without reported as spam.
>
> Whnever there are metrics, some will find a way to game the system to
> make theirs look better - this certainly isn't limited to github, or to
> tech, or in any way a recent thing.
>

Certainly true, and I think this is more of a social problem than a
technical one. If people are giving out review approvals to get more
points, you (where 'you' is a person with some privileges on the repo) can
click "dismiss review" and get rid of the noise, at least within that PR.
Maybe they still get points for the review, I'm not sure. Taking away the
ability for non-core contributors to offer official review approvals to
stop people like that only harms the people actually trying to do good work.

Gaming the system doesn't end up working well in the end anyway. The first
time the gamers try to get a job interview and can't explain how they'd do
a code review—something GitHub says they've done hundreds or thousands of
times—the whole thing will fail.
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/R3YU44XPWLBUWVLSYTTTWJZCSRRCB67F/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Guido van Rossum]

Where does it say that a review gives you points? The GitHub blog post I
saw about the subject only mentions commits.

On Mon, Jan 31, 2022 at 8:16 AM Brian Curtin  wrote:

> On Sun, Jan 30, 2022 at 8:42 AM Mats Wichmann  wrote:
>
>> On 1/30/22 04:45, Inada Naoki wrote:
>> > On Sun, Jan 30, 2022 at 7:37 PM Irit Katriel <
>> iritkatr...@googlemail.com> wrote:
>>
>> > Some people may do "approval without review" to make their "Profile"
>> > page richer, because GitHub counts it as a contribution.
>> > Creating spam issues or pull requests can be reported as spam very
>> > easily. But "approve without review" is hard to be reported as spam.
>> > So approving random issue is the most easy way to earn contributions
>> > without reported as spam.
>>
>> Whnever there are metrics, some will find a way to game the system to
>> make theirs look better - this certainly isn't limited to github, or to
>> tech, or in any way a recent thing.
>>
>
> Certainly true, and I think this is more of a social problem than a
> technical one. If people are giving out review approvals to get more
> points, you (where 'you' is a person with some privileges on the repo) can
> click "dismiss review" and get rid of the noise, at least within that PR.
> Maybe they still get points for the review, I'm not sure. Taking away the
> ability for non-core contributors to offer official review approvals to
> stop people like that only harms the people actually trying to do good work.
>
> Gaming the system doesn't end up working well in the end anyway. The first
> time the gamers try to get a job interview and can't explain how they'd do
> a code review—something GitHub says they've done hundreds or thousands of
> times—the whole thing will fail.
> ___
> Python-Dev mailing list -- python-dev@python.org
> To unsubscribe send an email to python-dev-le...@python.org
> https://mail.python.org/mailman3/lists/python-dev.python.org/
> Message archived at
> https://mail.python.org/archives/list/python-dev@python.org/message/R3YU44XPWLBUWVLSYTTTWJZCSRRCB67F/
> Code of Conduct: http://python.org/psf/codeofconduct/
>


-- 
--Guido van Rossum (python.org/~guido)
*Pronouns: he/him **(why is my pronoun here?)*

___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/WZOOZ5O6TDBISB3VQ7HTCMEUGGEV63A2/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Brian Curtin]

I was using points in a more generic sense, making your "contribution
activity overview" look nicer—I wasn't sure if "points" was an actual thing
or not, so maybe I'm speaking out of turn. Mine shows 70% of my actions are
code review, then issues, commits, and PRs are 10% each.

On Mon, Jan 31, 2022 at 9:40 AM Guido van Rossum  wrote:

> Where does it say that a review gives you points? The GitHub blog post I
> saw about the subject only mentions commits.
>
> On Mon, Jan 31, 2022 at 8:16 AM Brian Curtin  wrote:
>
>> On Sun, Jan 30, 2022 at 8:42 AM Mats Wichmann  wrote:
>>
>>> On 1/30/22 04:45, Inada Naoki wrote:
>>> > On Sun, Jan 30, 2022 at 7:37 PM Irit Katriel <
>>> iritkatr...@googlemail.com> wrote:
>>>
>>> > Some people may do "approval without review" to make their "Profile"
>>> > page richer, because GitHub counts it as a contribution.
>>> > Creating spam issues or pull requests can be reported as spam very
>>> > easily. But "approve without review" is hard to be reported as spam.
>>> > So approving random issue is the most easy way to earn contributions
>>> > without reported as spam.
>>>
>>> Whnever there are metrics, some will find a way to game the system to
>>> make theirs look better - this certainly isn't limited to github, or to
>>> tech, or in any way a recent thing.
>>>
>>
>> Certainly true, and I think this is more of a social problem than a
>> technical one. If people are giving out review approvals to get more
>> points, you (where 'you' is a person with some privileges on the repo) can
>> click "dismiss review" and get rid of the noise, at least within that PR.
>> Maybe they still get points for the review, I'm not sure. Taking away the
>> ability for non-core contributors to offer official review approvals to
>> stop people like that only harms the people actually trying to do good work.
>>
>> Gaming the system doesn't end up working well in the end anyway. The
>> first time the gamers try to get a job interview and can't explain how
>> they'd do a code review—something GitHub says they've done hundreds or
>> thousands of times—the whole thing will fail.
>> ___
>> Python-Dev mailing list -- python-dev@python.org
>> To unsubscribe send an email to python-dev-le...@python.org
>> https://mail.python.org/mailman3/lists/python-dev.python.org/
>> Message archived at
>> https://mail.python.org/archives/list/python-dev@python.org/message/R3YU44XPWLBUWVLSYTTTWJZCSRRCB67F/
>> Code of Conduct: http://python.org/psf/codeofconduct/
>>
>
>
> --
> --Guido van Rossum (python.org/~guido)
> *Pronouns: he/him **(why is my pronoun here?)*
> 
>
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/3Q7GEGI7JOCAEA7VNK7NVDSX2I43LFWD/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Guido van Rossum]

Ah, now I see the section on GitHub user home pages. Honestly if employers
just take a glance at that they get what they deserve. I don't want to
worry about this, there are enough real problems.

On Mon, Jan 31, 2022 at 8:48 AM Brian Curtin  wrote:

> I was using points in a more generic sense, making your "contribution
> activity overview" look nicer—I wasn't sure if "points" was an actual thing
> or not, so maybe I'm speaking out of turn. Mine shows 70% of my actions are
> code review, then issues, commits, and PRs are 10% each.
>
> On Mon, Jan 31, 2022 at 9:40 AM Guido van Rossum  wrote:
>
>> Where does it say that a review gives you points? The GitHub blog post I
>> saw about the subject only mentions commits.
>>
>> On Mon, Jan 31, 2022 at 8:16 AM Brian Curtin  wrote:
>>
>>> On Sun, Jan 30, 2022 at 8:42 AM Mats Wichmann  wrote:
>>>
 On 1/30/22 04:45, Inada Naoki wrote:
 > On Sun, Jan 30, 2022 at 7:37 PM Irit Katriel <
 iritkatr...@googlemail.com> wrote:

 > Some people may do "approval without review" to make their "Profile"
 > page richer, because GitHub counts it as a contribution.
 > Creating spam issues or pull requests can be reported as spam very
 > easily. But "approve without review" is hard to be reported as spam.
 > So approving random issue is the most easy way to earn contributions
 > without reported as spam.

 Whnever there are metrics, some will find a way to game the system to
 make theirs look better - this certainly isn't limited to github, or to
 tech, or in any way a recent thing.

>>>
>>> Certainly true, and I think this is more of a social problem than a
>>> technical one. If people are giving out review approvals to get more
>>> points, you (where 'you' is a person with some privileges on the repo) can
>>> click "dismiss review" and get rid of the noise, at least within that PR.
>>> Maybe they still get points for the review, I'm not sure. Taking away the
>>> ability for non-core contributors to offer official review approvals to
>>> stop people like that only harms the people actually trying to do good work.
>>>
>>> Gaming the system doesn't end up working well in the end anyway. The
>>> first time the gamers try to get a job interview and can't explain how
>>> they'd do a code review—something GitHub says they've done hundreds or
>>> thousands of times—the whole thing will fail.
>>> ___
>>> Python-Dev mailing list -- python-dev@python.org
>>> To unsubscribe send an email to python-dev-le...@python.org
>>> https://mail.python.org/mailman3/lists/python-dev.python.org/
>>> Message archived at
>>> https://mail.python.org/archives/list/python-dev@python.org/message/R3YU44XPWLBUWVLSYTTTWJZCSRRCB67F/
>>> Code of Conduct: http://python.org/psf/codeofconduct/
>>>
>>
>>
>> --
>> --Guido van Rossum (python.org/~guido)
>> *Pronouns: he/him **(why is my pronoun here?)*
>> 
>>
>

-- 
--Guido van Rossum (python.org/~guido)
*Pronouns: he/him **(why is my pronoun here?)*

___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/62USYC2MZK37QEP5RQKG3SGM2TBURM4S/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Lrupert via Python-Dev]

> Gaming the system doesn't end up working well in the end anyway. The first 
> time the gamers try to get a job interview and can't explain how they'd do a 
> code review—something GitHub says they've done hundreds or thousands of 
> times—the whole thing will fail.

Observably, it feels like they are doing this for core privileges (if they 
don't already exist, they are a member of the python org?). Every time I see 
one of those PRs (e.g add test for X, add delete redundant variables for Y), 
the author seem to be cc-ing their mentor. This gives a bad impression to 
others about their intentions (constant contribution of trivial / low quality 
stuff with little-to-no-gain to achieve a higher number of commits, since it is 
a visible metric).___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/JZBQ2UYTXDCHADW4LEPGPE5SFLRHW5E3/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Guido van Rossum]

Okay, now you might as well state which person you are talking about. Who
says their mentor hasn't encouraged them to do this?

On Mon, Jan 31, 2022 at 12:32 PM Lrupert via Python-Dev <
python-dev@python.org> wrote:

> > Gaming the system doesn't end up working well in the end anyway. The
> first time the gamers try to get a job interview and can't explain how
> they'd do a code review—something GitHub says they've done hundreds or
> thousands of times—the whole thing will fail.
>
> Observably, it feels like they are doing this for core privileges (if they
> don't already exist, they are a member of the python org?). Every time I
> see one of those PRs (e.g add test for X, add delete redundant variables
> for Y), the author seem to be cc-ing their mentor. This gives a bad
> impression to others about their intentions (constant contribution of
> trivial / low quality stuff with little-to-no-gain to achieve a higher
> number of commits, since it is a visible metric).
>
> ___
> Python-Dev mailing list -- python-dev@python.org
> To unsubscribe send an email to python-dev-le...@python.org
> https://mail.python.org/mailman3/lists/python-dev.python.org/
> Message archived at
> https://mail.python.org/archives/list/python-dev@python.org/message/JZBQ2UYTXDCHADW4LEPGPE5SFLRHW5E3/
> Code of Conduct: http://python.org/psf/codeofconduct/
>


-- 
--Guido van Rossum (python.org/~guido)
*Pronouns: he/him **(why is my pronoun here?)*

___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/EU3YC4IU3BYTHNZIT26QQOGYFYCAQZGJ/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Ethan Furman]

On 1/31/22 8:47 AM, Lrupert via Python-Dev wrote:

> This gives a bad impression to others about their intentions (constant 
contribution of trivial
> / low quality stuff with little-to-no-gain to achieve a higher number of 
commits, since it is
> a visible metric).

Two of us have already commented about the usefulness of those PRs and the fact that they have fixed previously unknown 
bugs -- those are not trivial, and certainly not low quality.


If you have a real concern, show us some examples of these useless PRs, 
otherwise let it be.

--
~Ethan~
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/EM25XB5XCJP4AG46GUR3ZKZJFCMQOUFQ/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Greg Ewing]

On 1/02/22 5:47 am, Lrupert via Python-Dev wrote:
This gives 
a bad impression to others about their intentions (constant contribution 
of trivial / low quality stuff with little-to-no-gain to achieve a 
higher number of commits, since it is a visible metric).


Or they may just feel that it's better to organise their changes into
a number of small, independent commits rather than one large one. I
wouldn't be too quick to jump to conclusions about people's motives
here.

--
Greg
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/4GZX7RHJKRRNAQSA45P7RNLHYB7F6Z6Y/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Barney Gale]

On Mon, 31 Jan 2022 at 23:29, Greg Ewing 
wrote:

> Or they may just feel that it's better to organise their changes into
> a number of small, independent commits rather than one large one. I
> wouldn't be too quick to jump to conclusions about people's motives
> here.


I've found that highly targeted PRs stand the best chance of getting
eyeballs/approvals/merges. It's not just preference - it's good sense.

And if folks want to "game" GitHub by spamming commits to an open source
project, CPython is a pretty poor choice! My occasional contributions can
take weeks or months to land due to the thorough review process and
peaks/troughs of core dev time for reviews.

Barney
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/VMBMUZUHTGBSIZY45YDHPULMRA6IY7TT/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Dong-hee Na]

If you feel bad impression, sorry about that.
The mentee who cc me is under mentoring period. Since tracking all of
mentee’s activity is impossible, I requested him to cc me.
This was for checking his labeling is valid or not.

Warm regards
Dong-hee
2022년 2월 1일 (화) 오전 5:35, Lrupert via Python-Dev 님이
작성:

> > Gaming the system doesn't end up working well in the end anyway. The
> first time the gamers try to get a job interview and can't explain how
> they'd do a code review—something GitHub says they've done hundreds or
> thousands of times—the whole thing will fail.
>
> Observably, it feels like they are doing this for core privileges (if they
> don't already exist, they are a member of the python org?). Every time I
> see one of those PRs (e.g add test for X, add delete redundant variables
> for Y), the author seem to be cc-ing their mentor. This gives a bad
> impression to others about their intentions (constant contribution of
> trivial / low quality stuff with little-to-no-gain to achieve a higher
> number of commits, since it is a visible metric).
>
> ___
> Python-Dev mailing list -- python-dev@python.org
> To unsubscribe send an email to python-dev-le...@python.org
> https://mail.python.org/mailman3/lists/python-dev.python.org/
> Message archived at
> https://mail.python.org/archives/list/python-dev@python.org/message/JZBQ2UYTXDCHADW4LEPGPE5SFLRHW5E3/
> Code of Conduct: http://python.org/psf/codeofconduct/
>
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/OHVDUMMSGVFWIFJ3K46OWNY6OURK3XI6/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Nikita Sobolev]

Hi, my name is Nikita and I think that I am the person behind these spammy PRs.
Link: https://github.com/python/cpython/pulls/sobolevn

First of all, Lrupert, sorry for all the noise and inconvenience I caused to 
you personally and to other community members! This totally was **not** my 
intention. 

You could have reached out to me earlier via email or directly in some PR to 
share your concerns, maybe we could have this whole situation avoided.

Secondly, thanks for letting me know about how others might feel about my work. 
Feedback is always important. I hope I can learn a lot from this case.

But, I can tell you honestly that I've never been to a situation like this 
before and I don't know exactly how to handle it and what to improve in this 
specific case.

Third, I agree that almost all my PRs were about some trivial things. Mostly 
because I was reading through typeshed code (which requires looking through 
multiple CPython modules at a very high level) and test code for these modules 
where I've spotted quite a lot of details to fix.

I cannot obviously judge the quality of my work (except for being ok-ish), so I 
will leave this part out of scope. The only thing I can say here is that it's a 
bit sad thread to read on python-dev mailing list, but I will keep my optimism 
for the future :)

So, to sum things up:
- I am open to any feedback: if others also think that my work does not bring 
any value - this is fine! I can totally improve or work on something simpler / 
some other project. Anyone interested can write me a direct email: 
m...@sobolevn.me
- I am sorry for causing this thread. It is far from being a pleasant or 
technical read

Best Regards,
Nikita Sobolev
https://github.com/sobolevn
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/LF7V3ZASXK6DGD5MBBXP3YKHGOLW36D5/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Increase of Spammy PRs and PR reviews

[Terry Reedy]

On 1/31/2022 7:31 PM, Nikita Sobolev wrote:

Hi, my name is Nikita and I think that I am the person behind these spammy PRs.
Link: https://github.com/python/cpython/pulls/sobolevn


Nikita, I don't know if the OP was responding only to your PRs or 
others, but I other people have seen truly trivial PRs from other 
people.  Example: just after this thread started, someone opened an 
issue and a PR with news item to change an IDLE test name from 
'test_formated' to 'test_formatted'.  (I considered rejecting as 'too 
trivial'.  But since the person has been active on multiple projects in 
the last year and signed the CLA, I expect to eventually merge the PR.)


Reviews consisting only of LGTM are a separate but real issue.  I find 
them useless from unknown newcomers, but do not want any discouragement 
of real reviews, even to point out a one misspelling or suggest one 
better wording.


In any case, I was one of those who approved the idea that it should be 
possible to run files with a main clause both with a standard command 
line and from an editor designed to run a file as 'main'.  I also 
encouraged multiple easily reviewable PRs from you.  Please continue.



--
Terry Jan Reedy

___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/QJ7ZWIRRRDQKKBJ24IEOX2RF7LWFUTRU/
Code of Conduct: http://python.org/psf/codeofconduct/