Skip to content
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

PDEP-1 Revision (Decision Making) #53576

Merged
merged 19 commits into from
Apr 1, 2024
Merged

Conversation

noatamir
Copy link
Member

@noatamir noatamir commented Jun 9, 2023

The governance working group has been discussing the PDEP workflow and decision-making for some time now. Feedback is welcome.

This PR proposes some formulation of the existing workflow to clarify and facilitate it for current and future authors, and voting members.
In our discussions, we assessed that a more structured process would help folks who submit the PDEP know how and when it can progress, and how to better engage with others.

The timelines were considered to facilitate an engaging discussion, and automation. We aim to make the lives of the PDEP authors easier by taking away the need to decide "has it been long enough", or "cat herding" folks to engage and clarify their comments so that PDEPs don't get large change requests that could have been clarified early on - late in the process.

The quorum was discussed a lot in our working group, and we're unsure about the best strategy here. We were looking for a sufficient number so if a decision is passed one feels it's valid (e.g. maybe 2 people is too low), but not too high so that we feel like ongoing discussions are blocked because it's too hard to get that many voting members involved. Further proposal on this point are welcome.

We tried to make things as flexible and clear as possible. Being mindful of the needs of the authors, as well as those engaging in the discussion, be them voting members or not.

At this point, we would like to invite feedback. It might be that you need more clarity on the suggestions, have ideas for improvement, or aren't convinced that this will affect the current process positively.

Co-authored-by: @MarcoGorelli, @rhshadrach, @Dr-Irv, @jorisvandenbossche

@noatamir noatamir requested a review from datapythonista as a code owner June 9, 2023 15:32
@noatamir noatamir requested a review from a team June 9, 2023 15:33
Copy link
Contributor

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some small things

web/pandas/pdeps/0001-purpose-and-guidelines.md Outdated Show resolved Hide resolved
web/pandas/pdeps/0001-purpose-and-guidelines.md Outdated Show resolved Hide resolved
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice - lgtm

@topper-123
Copy link
Contributor

+1 from me.

#### Casting Votes

As the voting period starts, a VOTE issue is created which links to the PDEP discussion pull request.
Each voting member may cast a vote by adding one of the following comments:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies if I havce missed this but it does appear that a "voting member" is explicitly defined?

The removed section seems to suggest that a voting member is anyone in the "core development team". Does pandas have any segregation between "active" and "inactive" core development members and does this have any bearing on their right to vote?

Is there any scope to specifically invite certain informed (non-core dev) parties to vote on issues? I would probably consider this an abuse of power and instead invite to participate in the discussion to influence members.

If votes are cast by core dev members does this have implications for inviting future members into the core dev team? i.e to not push for adding members of a group of similar, or friendly like minded individuals, perhaps from the same institution etc, that can build up substantial iunfluence?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bit of a chicken and egg issue here. We are planning on updating other governance documents to define who can vote. Right now, it is the core team, but it is likely we will not use that in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, it is the core team, but it is likely we will not use that in the future.

IMO we should also require a discussion to change this, hence my comment to define voting member in this PDEP now.

- Reason: A one sentence reason is required.
- -1: disapprove
- Reason: A one sentence reason is required.
A disapprove vote requires prior participation in the PDEP discussion issue.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be sufficient for a voting member to follow a discussion led by knowledgeable parties and simply provide a vote that read: "-1: disapprove. I agree with XXX's analysis and concerns and disagree that YYY's argument and the PDEP are suitable".

Rather than provide rules that restrict voting in certain ways (which might be difficult to police anyway), would it not be sufficient to design quorum and majority rules that aim to progress PDEPs that have garnered 'sufficient' support (whatever that may end up being), above objections.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be sufficient for a voting member to follow a discussion led by knowledgeable parties and simply provide a vote that read: "-1: disapprove. I agree with XXX's analysis and concerns and disagree that YYY's argument and the PDEP are suitable".

They could post that in the PDEP discussion issue, and then cast their vote that way in the voting issue.

Rather than provide rules that restrict voting in certain ways (which might be difficult to police anyway), would it not be sufficient to design quorum and majority rules that aim to progress PDEPs that have garnered 'sufficient' support (whatever that may end up being), above objections.

I think our goal here was to avoid the case where we have one or two people who oppose a PDEP stop the PDEP from moving forward. If you have an idea on how to improve what we've proposed, we'd certainly be open to that.

@mroeschke mroeschke added the PDEP pandas enhancement proposal label Jun 14, 2023
Each voting member, including the author(s) may cast a vote by adding one of the following comments:

- +1: approve.
- 0: abstain.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add a small explanation of how we understand "abstaining" or what it could mean?
Something like "Not fully convinced, but don't want to block it", or better worded (although there might be many reasons to vote +0, like "I haven't closely followed, but trust the majority")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that the one sentence reason thing on the next line?
Formally, it is anyone who wants the vote to proceed (achieve quorum), but not interested in voting for or against it. But that's kind of the dictionary definition almost no?!

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @noatamir generally lgtm. Should probably have a section on voting members even if for now is a one-liner... active members of the core team. (active/inactive has been defined/agreed elsewhere?)

web/pandas/pdeps/0001-purpose-and-guidelines.md Outdated Show resolved Hide resolved
web/pandas/pdeps/0001-purpose-and-guidelines.md Outdated Show resolved Hide resolved
web/pandas/pdeps/0001-purpose-and-guidelines.md Outdated Show resolved Hide resolved
@jorisvandenbossche
Copy link
Member

As far as I remember, the main point of contention last summer was the exact threshold to require for acceptance. The text currently says "a majority of 75% of the non-abstaining votes", but there were some in favor of lowering this threshold.

I think the main relevant discussion on this PR starts at #53576 (comment), continuing in the comments below.

I started a poll with the core team to get an idea of what threshold would be most acceptable.

@simonjayhawkins
Copy link
Member

Thanks @jorisvandenbossche

Great to know the process hasn't been abandoned.

I think that the PDEP process (which is ironically this PR) should probably include processes for the scenarios where the PDEP process doesn't follow the suggested timescales and appears abandoned. This also applies to VOTE issues that is not closed after the voting period ends. (for instance do votes still count until the issue is closed?)

A PDEP or PDEP revision that is not explicitly rejected is also not accepted.

I think we need to be able to close stale PDEP PRs or voting issues rather than leaving open with zero communication so that it is clearer to the community what the status is.

Comment on lines 96 to 102
To enable and encourage discussions on PDEPs, we follow a notification schedule. At each of the
following steps, the pandas team, and the pandas dev mailing list are notified via GitHub and
E-mail:
- Once a PDEP is ready for discussion.
- After 30 discussion days, with 30 days remaining for discussion.
- After 45 discussion days, with 15 days remaining for discussion.
- In case 15 days passed without any new unaddressed comments, the authors may close the discussion period
Copy link
Contributor

@Dr-Irv Dr-Irv Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To enable and encourage discussions on PDEPs, we follow a notification schedule. At each of the
following steps, the pandas team, and the pandas dev mailing list are notified via GitHub and
E-mail:
- Once a PDEP is ready for discussion.
- After 30 discussion days, with 30 days remaining for discussion.
- After 45 discussion days, with 15 days remaining for discussion.
- In case 15 days passed without any new unaddressed comments, the authors may close the discussion period
To enable and encourage discussions on PDEPs, we follow a notification schedule. At each of the
following steps, the pandas team, and the pandas dev mailing list are notified via GitHub and
E-mail, with an indication that a voting period will commence after 15 days of the notification date if there is no further discussion:
- Once a PDEP is ready for discussion.
- After 30 discussion days, with 30 days remaining for discussion.
- After 45 discussion days, with 15 days remaining for discussion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dr-Irv I pushed a different commit with an attempt to address the same thing, see 88a7db9. Can you take a look at that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jorisvandenbossche I looked at just that commit and made a comment on that text.

@jorisvandenbossche
Copy link
Member

@pandas-dev/pandas-core please take a final look / provide final comments, or otherwise (re-)approve the PR!

Main changes since last discussion round:

  • Changed the threshold from 75% to 70%
  • In the discussion timeline, tweaked the language around an early vote if there was no more discussion for 15 days (after a minimum of 30 days)

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no objections

@jorisvandenbossche
Copy link
Member

One more small change: I added an extra "Withdrawn" status in addition to "Rejected", for the case the author(s) themselves decide to not get to a vote if they are convinced from the discussion it's eventually not a good idea or when there is an alternative proposal that is decided to push forward, but we still want to merge the PDEP to document the discussion (this status is also used in PEPs, and also numpy and other projects).

@jorisvandenbossche
Copy link
Member

A final reminder here, I plan to merge this by Friday, assuming we have consensus on those changes.

@mroeschke mroeschke merged commit 95f911d into pandas-dev:main Apr 1, 2024
17 of 18 checks passed
@mroeschke
Copy link
Member

Thanks all. Seems like we have consensus here.

@jorisvandenbossche
Copy link
Member

And thanks @noatamir and the initial write-up, and everyone for the feedback!

pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
* adding voting draft

* new workflow intro

* update after feedback

* fix errors

* fix errors

* Apply suggestions from code review

Co-authored-by: Irv Lustig <[email protected]>

* clarification

* updated based on feedback

* updated based on feedback

* typo

* Updated following Simon's feedback

Co-authored-by: Simon Hawkins <[email protected]>

* Apply suggestions from code review

Co-authored-by: Irv Lustig <[email protected]>

* change majority to 70%

* some small formatting changes

* reword case of early vote

* use Irv's suggestion to reword notifications

* add withdrawn status

---------

Co-authored-by: Irv Lustig <[email protected]>
Co-authored-by: Simon Hawkins <[email protected]>
Co-authored-by: Joris Van den Bossche <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PDEP pandas enhancement proposal Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.