heliophysicsPy / standards

3 stars 11 forks source link

PHEP 1: PHEP Purpose and Guidelines #22

Closed jtniehof closed 6 months ago

jtniehof commented 1 year ago

tl;dr

If this is all a bit overwhelming, you can look at the file view to see the full proposed text. You can comment line-by-line and reply to others' line-by-line comments there. The Markdown source is pretty human-readable without rendering.

Overview

This PR proposes a process by which the PyHC community makes collective decisions. Ideally everything needed to understand this is within the document itself. This incorporates changes due to the conversation on the PyHC call 2023-10-16 (slides) and 2023-11-27 (slides)

There are some ongoing conversations in comments on the applicable lines. Some are linked below, as well as some questions not relating to specific lines.

@sapols, I ask that you take the role of editor for this one, just check for formatting and consistency as described herein?

Renders, diffs, etc.

I am no longer doing the diffs back to the dawn of time, as they're basically unreadable anyway.

The diffs were wrapped after diffing, so the columns don't line up perfectly...hopefully it's still readable.

Models

PHEP-1 was primarily based on PEP-1, with consideration of NEP-0 and SEP-1. Similar processes:

Open questions and comments

Also see #25 for discussion of a template.

Resolved questions and comments

I believe these issues have been resolved in discussion, but they can be revisited if anybody objects to the resolution.

Other notes

Input on nice Markdown editors is always welcome; here are a few:

BaptisteCecconi commented 1 year ago

Hi @jtniehof, you can add: VEP in you list (IVOA Vocabulary Enhancement Proposal).

sapols commented 1 year ago

I second Nick, thanks so much @jtniehof! Sorry for the delay; I skimmed the document and conversation, and I have lots of thoughts to add. It's the end of my workday though, so rather than rush now, I'll add my comments tomorrow so I can take my time and be thorough.

sapols commented 1 year ago

Or, you know, sometimes inspiration strikes at midnight and that's valid too 😂

sapols commented 1 year ago

To directly address some of the open questions:

Lastly, about markdown editors: I do a lot of development in the PyCharm IDE, and I think its built-in markdown renderer is perfectly good.

sapols commented 1 year ago

Thank you so much for doing this! I think it's really in good shape.

The main thing I'm wondering is...are there ways that we could streamline the process or make it less prescriptive? For example, would creating a template for PHEPs let us avoid the need to describe the formatting details? We have a detailed process for PLEPs for PlasmaPy, but we've usually ended up going through the process less formally since usually it's a discussion among a small group of people.

In any case, I really do like how this is coming along and appreciate the work that you put into it. Thank you once again!

@namurphy point taken. Now that I compared this to PLEP-1 and SEP-1, this PHEP-1 is much longer.

jtniehof commented 1 year ago

Reply to

The main thing I'm wondering is...are there ways that we could streamline the process or make it less prescriptive? For example, would creating a template for PHEPs let us avoid the need to describe the formatting details?

Any formatting details that we do need should be documented independent of their examples, but I agree a template would help people not worry about it.

I kept PEP-1 as much as possible for a few reasons:

  1. Credibility: I can blame any problems on someone else :) and I think (hope) we can all agree it's a reasonably legitimate process
  2. Scope: PyHC is smaller than Python, but (I think we can agree) bigger than say SunPy, so a streamlined process that works on a specific project might be too streamlined for a community of this size and breadth

I still think the process is about right, not too heavy. But the document is too big. Looks like two problems:

  1. The list of parts/sections is too long, too prescriptive, and doesn't even match the actual parts in PHEP-1
  2. It's just plain too chatty

Before my next push with the specific edits requested, I'll go through and try to tighten up in general, and in particular rework "What belongs in a successful PHEP?" so that it's more template-able and so that PHEP 1 matches it.

jtniehof commented 1 year ago
* We should have an index of PHEPs. I can add the initial (nearly degenerate) table as part of this PR, or we can take a different approach (automatically indexed? if so, just on the website and not in the repository?)
  **_- The website seems like a natural place to put this index. But what's this nearly degenerate table?_**

It's just that the current list of PHEPs is going to be very short :)

jtniehof commented 1 year ago

New update pushed, see top comment for various links. @sapols , do you agree that this is consistent with the "Submitting a PHEP" section to the extent it can be considered "approved" as a Draft PHEP, and (confusing wording aside), the PR marked ready-for-review (not draft)?

That doesn't mean I'm submitting it for acceptance, just that the general editorial stuff up to what would ordinarily be a number assignment is compliant.

jtniehof commented 1 year ago
  • Do we want to have an autobuilder that renders PHEPs to the website or just link the index to this repository? It looks like GH doesn't render embedded SVGs. - I think we gotta make a decision about the index before we can decide this. Although, GitHub not rendering embedded SVGs has a simple fix: use PNGs instead.

I was wrong...it does render them, it just looks terrible in dark mode! Black-on-dark-grey.

sapols commented 1 year ago

New update pushed, see top comment for various links. @sapols , do you agree that this is consistent with the "Submitting a PHEP" section to the extent it can be considered "approved" as a Draft PHEP, and (confusing wording aside), the PR marked ready-for-review (not draft)?

That doesn't mean I'm submitting it for acceptance, just that the general editorial stuff up to what would ordinarily be a number assignment is compliant.

@jtniehof yes I agree. 👍

jtniehof commented 11 months ago

All comments and open issues have been resolved, so this is likely the version that will go to a vote unless there is further input.

rebeccaringuette commented 10 months ago

Happy to comment, but I would prefer to add comments when there is a bit more of a complete picture. @namurphy do you think you can add some details to the right side of Jon's table? And maybe tag me when you are done?

rebeccaringuette commented 10 months ago

Please tag me when @Cadair and @namurphy are done and I will take another look.

namurphy commented 10 months ago

Thank you for the summary above, @jtniehof! I'll first provide a few practical examples why we might want to amend PHEPs. In all of these cases, the central ideas of the PHEP are preserved. Some of the changes are non-substantive, while others have some minor change to the meaning.

  1. A link to a reference in a PHEP breaks due to link rot and we need to update it.
  2. We include an implementation detail in a PHEP, and the technology later changes.
    • This occurred in Astropy when they approved an APE to use black as an autoformatter, which prevented them from switching to the ruff formatter which has considerably better performance (https://github.com/astropy/astropy-APEs/pull/90).
  3. We want to make a minor change to the process for approving PHEPs or governance.
  4. We need to fix a typo.
  5. We find a formatting error.
  6. We discover a factual error that we want to fix.
  7. We approve a PHEP about package interoperability, and packages move to implement it. During the implementation process, we discover an edge case that the PHEP is ambiguous about.
  8. We decide to switch from, for example, GitHub flavored Markdown to MyST flavored Markdown.
  9. During the process of implementing a PHEP across packages, a number of pull requests are made. We want to add links to these pull requests under a section called "Related pull requests".
  10. A PHEP gets superseded, and we want to change its metadata to indicate that it has been superseded, and add a note describing how it has been superseded.
  11. We want to add a forward reference to a newer related PHEP.
  12. We add a new pre-commit hook that pretty-formats Markdown files.
nabobalis commented 10 months ago

My view on those is that any text changes that are like style or typos would be allowed or untaken by the editor(s). I can't imagine that those going any review process.

namurphy commented 10 months ago

My view on those is that any text changes that are like style or typos would be allowed or untaken by the editor(s). I can't imagine that those going any review process.

Agreed! PlasmaPy's process allows non-substantive changes like typo/formatting fixes to go through a streamlined process. They still require approval of the Coordinating Committee but wouldn't have to be announced to the community, etc.

rebeccaringuette commented 10 months ago

Yes, minor changes should not need review, but changes to the content should get reviewed (e.g. #s 2, 3?, 6, and 7).

On Wed, Jan 24, 2024 at 2:07 PM Nick Murphy @.***> wrote:

My view on those is that any text changes that are like style or typos would be allowed or untaken by the editor(s). I can't imagine that those going any review process.

Agreed! PlasmaPy's process allows non-substantive changes like typo/formatting fixes to go through a streamlined process. They still require approval of the Coordinating Committee but wouldn't have to be announced to the community, etc.

— Reply to this email directly, view it on GitHub https://github.com/heliophysicsPy/standards/pull/22#issuecomment-1908752607, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALX7QXRC5PSLZOGNQ5VKN3DYQFLXDAVCNFSM6AAAAAA6N54NPGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYG42TENRQG4 . You are receiving this because you were mentioned.Message ID: @.***>

namurphy commented 10 months ago

To follow up a bit more, my suggested scheme for mutable PHEPs and amendments would be along the lines of:

  1. Make each PHEP a versioned document. We could do this by uploading each PHEP to Zenodo, and then uploading a new version of that record.
  2. Create a streamlined process for changes that do not introduce any substantive changes. This would include fixing typos, updating links/metadata, and updating contextual information (like related issues and pull requests).
    1. Create a process for amending PHEPs. My preference would be for this process to also be streamlined while maintaining the same acceptance criteria, but we could make it so that amendments have to go through the full approval process.

A related possibility would be to distinguish between the Accepted and Final statuses which is closer to the system that PEPs use. Once PEPs are "Accepted", they often still undergo changes & iterations during the implementation process. When they are "Final", then they are rarely changed afterwards in a substantive way.

[I'll update this post later with a bit more discussion about why I'm suggesting mutability of PHEPs.]

jtniehof commented 10 months ago

Those are great exmaples @namurphy, and span a pretty good range. It appears everybody has a different idea of what a "revision" might mean. I'm concerned about having three possible processes: new / replacement, revision that requires review, and revision that doesn't require review.

What I suggest is that I write up a candidate language for revision that doesn't require review--link rot and typo type stuff. Everybody can review that for reasonableness in its own right, and see if that plus the replacement covers our use cases adequately.

My personal bias is towards fewer processes that may occasionally not be a perfect fit for the situation, rather than having lots of processes to try and fit everything perfectly. Another personal bias is towards writing standards to be flexible with regard to implementation and change where it can be done unambigously--you can see some of that here where there's mention of "PyHC leadership". Would it make sense to write that flexibility into PHEP-1 as a recommendation?

As regards implementations, after some discussion I changed the draft to require a reference implementation before the vote (unlike PEP-1, which allows "Approved" before implementation, PHEP-1 removes the "Approved" state). This should help minimize cases where something is fully voted on and Final but the implementation runs into problems.

rebeccaringuette commented 10 months ago

Drafting the candidate language for revision sounds like a good next step. The flexibility of the implementation of a standard... that needs discussion. My guess is the flexibility of a given standard will depend on the given standard. Perhaps the PHEP for a given standard should require a few sentences on how flexible the standard is meant to be.

On Fri, Jan 26, 2024 at 9:21 AM Jon Niehof @.***> wrote:

Those are great exmaples @namurphy https://github.com/namurphy, and span a pretty good range. It appears everybody has a different idea of what a "revision" might mean. I'm concerned about having three possible processes: new / replacement, revision that requires review, and revision that doesn't require review.

What I suggest is that I write up a candidate language for revision that doesn't require review--link rot and typo type stuff. Everybody can review that for reasonableness in its own right, and see if that plus the replacement covers our use cases adequately.

My personal bias is towards fewer processes that may occasionally not be a perfect fit for the situation, rather than having lots of processes to try and fit everything perfectly. Another personal bias is towards writing standards to be flexible with regard to implementation and change where it can be done unambigously--you can see some of that here where there's mention of "PyHC leadership". Would it make sense to write that flexibility into PHEP-1 as a recommendation?

As regards implementations, after some discussion I changed the draft to require a reference implementation before the vote (unlike PEP-1, which allows "Approved" before implementation, PHEP-1 removes the "Approved" state). This should help minimize cases where something is fully voted on and Final but the implementation runs into problems.

— Reply to this email directly, view it on GitHub https://github.com/heliophysicsPy/standards/pull/22#issuecomment-1912144764, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALX7QXRNDHHGZUCJ3AD3QJ3YQO3VJAVCNFSM6AAAAAA6N54NPGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJSGE2DINZWGQ . You are receiving this because you were mentioned.Message ID: @.***>

jtniehof commented 9 months ago

I've added the revision process. It probably makes most sense to look at the full diff because the effects are pretty wide-ranging, but you can also see the elements of:

I've also made sure "revise" and "revision" refer only to this process.

I am not entirely satisfied with this. It's not clear that it saves much effort, as there's still PR, status updates, DOI minting, etc.--a lot of that was probably implicit in just being able to replace a PHEP, made explicit here. (Although maybe those were dodgeable--do we need to mint a new DOI for a PHEP just to indicate it's been replaced?)

If there are really non-controversial changes, one would also think they'd sail through two rounds of voting and we wouldn't hurt much for the two months' wait.

I've honestly done the best I can on this, and I'm just not seeing a good path for substantive changes. For the most part, if somebody wants to see something different, I'm going to need suggested wording. I'd also be open to input on the summary in the "open issues" section on this, which will hopefully eventually go back into rejected ideas (in some form, based on what exactly gets rejected).

rebeccaringuette commented 9 months ago

Thanks, @jtniehof. Your effort has been substantial and worthwhile. This work is definitely ready for an in-depth discussion at the next PyHC meeting towards the goal of adoption. @jibarnum is there room in the schedule for this?

sapols commented 9 months ago

@jtniehof I'm completely satisfied with the new language, thanks for all your work. I think keeping them "largely immutable" with the revision process you outlined for trivial changes is perfect. Once that one typo gets fixed ("data" instead of "date"), I'm finally ready to hit the approve button.

If others are satisfied, I'd suggest we move to discuss formally voting on this PHEP.

rebeccaringuette commented 9 months ago

Sounds good to me.

On Thu, Feb 8, 2024 at 2:48 PM Shawn Polson @.***> wrote:

@jtniehof https://github.com/jtniehof I'm completely satisfied with the new language, thanks for all your work. I think keeping them "largely immutable" with the revision process you outlined for trivial changes is perfect. Once that one typo gets fixed ("data" instead of "date), I'm finally ready to hit the approve button.

If others are satisfied, I'd suggest we move to discuss formally voting on this PHEP.

— Reply to this email directly, view it on GitHub https://github.com/heliophysicsPy/standards/pull/22#issuecomment-1934828510, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALX7QXWOZCWZ2HKGJOBRWVDYSUTW7AVCNFSM6AAAAAA6N54NPGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZUHAZDQNJRGA . You are receiving this because you were mentioned.Message ID: @.***>

jtniehof commented 8 months ago

I've made minor cleanups per comments here; those are pushed, and the template in PHEP-2 is also updated in #25.

jtniehof commented 8 months ago

Mea culpa...I announced on the PyHC list and Slack that we will be voting on this at the spring meeting, but didn't mention it here. See the spring meeting agenda.

jtniehof commented 7 months ago

I will make the changes with approval and DOIs and push, then request merge. I'll update #26 as necessary.

jtniehof commented 6 months ago

I believe we are now ready to merge and mark a release. @sapols, please merge and tag a release. See line 27 of https://github.com/heliophysicsPy/standards/pull/26/files#diff-2b1b69303b927a484e02c7fad9fc87d0d3ff0dc22ae1da0ecd0dc935d922a23c for the release creation. Attached is the final PHEP to use as the release file: phep-0001.pdf

jtniehof commented 6 months ago

We are done! Zenodo record and GitHub release