PDEP-1 first revision (scope) #51417
Conversation
|
Thanks @Dr-Irv! I'm fixing it up. Should pandas-stubs also be |
Yes. And I may have missed a few other places where backquotes are needed. |
Yeah, I'm adding a bunch more following your examples =) |
Let me know when you're done with your edits, and I'll take another look. Thanks! |
|
I haven't been to a governance meeting in a while(had a time conflict), but is there a procedure for revising a PDEP/deferring parts of a PDEP? I am asking since I am going to open PDEP-7 soon, and there are some parts where I would like to survey users, but not have that block acceptance of a PDEP. It would also be nice to have since this PR itself revises a PDEP :). |
Looking through the existing text, it looks like there's some process for this, but it would be nice to clarify whether changes would need to be accepted again and who can propose revisions. |
|
Nice, thanks for writing this up! 🙌 |
I believe that as long as you mean changes during the discussion phase, you don't need a process for amending your PDEP. Only once it has been accepted you'd need to follow a revision process. We are working on a definition for the decision making process (e.g. voting and acceptance) at the moment. |
jorisvandenbossche
changed the title
There is indeed some text around revisions currently. But it's maybe something we should discuss a bit more to what extent small revisions (after acceptance) would need to follow the full normal decision process. |
|
Preview docs at: https://pandas.pydata.org/preview/51417/ |
Why? PDEPs are predominantly to engage the community in significant changes to pandas? So this should also apply to PDEP revisions too? It's not clear to me what the motivation is for revising the PDEP. |
|
It's just to clarify a few things
That, and to have a process with which to make significant changes |
Thanks @MarcoGorelli I think the bar for revising a PDEP should be as high as the initial PDEP itself and therefore the motivation for the changes should be transparent (to the community). maybe the revision status should indicate what needed clarifying. And also the change to requiring core-team sponsors explicitly mentioned? |
|
Sure, good point For context, the revision comes out of the biweekly governance meeting, quite some discussion went into it And yes a brief summary of the revision would be good (e.g. in pdep4 there's a sentence to say why it was revised, even though that particular revision was really minor) |
Yes. I do read the minutes. But we are now transferring that outcome to the wider community. We introduced the PDEP as a formal process for pandas changes. I don't want to see that process compromised by allowing or encouraging revisions that would not have the same scrutiny or oversight. (to be clear. I'm not trying to block the changes in this PR per se.) |
|
The top post could indeed have provided a brief summary and rationale of the proposed changes, to give some context to the diff. Now, I think apart from clarification of existing text (what Marco mentioned), the "sponsor" seems to be the only substantial change. Potential summary:
The "sponsor" term is inspired on the PEP process (https://peps.python.org/pep-0001/#submitting-a-pep), but we should probably explain that in a bit more detail in the text here. @noatamir if that sounds about right, I (or you) can update the top post to include this summary.
The first PDEP-1 was done as a PR, this revision is using the exact same process: here is a PR with proposed changes that is open for discussion and needs to be approved by the core team. |
|
I understand your concerns Brock, and if the suggestion was that any behaviour change warranted a PDEP, then that would indeed be absurd. But if a change is fundamental, then it could benefit from the PDEP process. In #52140 specifically, the issue starts with "These changes are somewhat fundamental", and so I'm inclined to agree that it should be a PDEP. Regarding the changes being proposed in this PR - are you on board with them? If not, that's fine, but could you please point out which specific parts you'd like to see changed? Looking forward, we're going to have to make big decisions around nullable dtype and pyarrow, which I know you care about. By improving governance and decision-making, the expectation is that we will all together be able to make pandas even better |
Co-authored-by: Terji Petersen <contribute@tensortable.com>
|
@jbrockmendel can you respond to Marco's answer? Does that answer your concern, or do you still have concrete feedback for this PR? |
|
The problematic sentence is not made worse by this PR, so no objection. I only commented here because irv pointed to the PR specifically as relevant. |
| ### Clear examples for potential PDEPs: | ||
|
|
||
| - Adding a new parameter to many existing methods, or deprecating one in many places. For example: | ||
| - The `numeric_only` deprecation affected many methods and could have been a PDEP. |
There was a problem hiding this comment.
should this have a GH ref like most of the others?
| - The `numeric_only` deprecation affected many methods and could have been a PDEP. | ||
| - Adding a new data type has impact on a variety of places that need to handle the data type. | ||
| Such wide-ranging impact would require a PDEP. For example: | ||
| - `Categorical` ([GH-7217][7217], [GH-8074][8074]), `StringDtype` ([GH-8640][8640]), `ArrowDtype` |
There was a problem hiding this comment.
I don't directly find a clear issue for ArrowDtype, only various PRs implementing parts of it.
There was a problem hiding this comment.
no probs. not a blocker.
| - On the other hand, `DataFrame.assign()` might. While it is a single method without backwards | ||
| compatibility concerns, it is also a core feature and the discussion should be highly visible. | ||
| - Deprecating or removing a single method would not require a PDEP in most cases. For example: | ||
| - `DataFrame.xs` ([GH-6249][6249]) is an example of deprecations on core features that would be |
There was a problem hiding this comment.
this should maybe be a "On the other hand" case with an additional example that supports the "Deprecating or removing a single method would not require a PDEP" case. (to be consistent with the "Adding new methods or parameters to an existing method typically will not require a PDEP" examples above.)
There was a problem hiding this comment.
Good point. I would maybe change the example to DataFrame.append deprecation, which I think was a clear example of a controversial deprecation (at least for users) that already happened in the past (while I think for xs there is no much active discussion about it, the linked github issue's last activity is from a few years ago)
Co-authored-by: Simon Hawkins <simonjayhawkins@gmail.com>
Co-authored-by: Simon Hawkins <simonjayhawkins@gmail.com>
| - Deprecating or removing a single method would not require a PDEP in most cases. For example: | ||
| - That said, `DataFrame.append` ([GH-35407][35407]) is an example of deprecations on core |
There was a problem hiding this comment.
perhaps also need to remove the For example: to make this scan better if not adding a case where a PDEP is not required.
|
Is there anything stopping this being merged now? (we don't have the voting procedure until the next revision of this document is accepted) |
|
No, after the edit for your last comment yesterday, I was just waiting for CI to pass ;) So going to merge now. |
|
Thanks @noatamir and others. |
|
Thanks @noatamir for leading the effort, and thanks to everyone who contributed feedback both in the meetings as here on the PR! |
This proposal revises the PDEP definition and guidelines. Feedback is welcome.
Context and summary of the changes:
Co-authored-by: @MarcoGorelli, @rhshadrach, @Dr-Irv, @jorisvandenbossche, @mroeschke