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

SymPEP Purpose and Process and SymPEP Template #2

Merged
merged 16 commits into from
Sep 18, 2023

Conversation

asmeurer
Copy link
Member

I have started SymPEP 1 as well as a template for SymPEPs. This is heavily based on NEP 0 (which is itself based on PEP 1. The template is based on the NEP template. However, I have made substantial changes, including removing some things that seemed unnecessary, and adding some comments about things that I think were not clearly stated.

This is very much a first draft. Please feel free to comment on any of the specifics here. If you feel the process outlined here is missing something, or is wrong, please let me know.

This is based in large part on the NEP template
https://numpy.org/neps/nep-template.html, with some additions.
This is based in large part on NEP 0, which is itself based on PEP 1. However,
there are significant deviations from both.
@asmeurer
Copy link
Member Author

asmeurer commented Jan 27, 2021

One issue here which I haven't yet resolved is that there are three types of SymPEPs outlined, "standards track", "informational", and "process", but a lot of the SymPEP 1 text, and most of the template actually only applies to "standards track".

(I'm also not a fan of the term "standards track", as it sounds too formal, but I can't think of a better term right now)

@asmeurer
Copy link
Member Author

Another thing that's missing is a discussion on what sorts of changes deserve a SymPEP. The reason is I'm not really sure about what that should be.

SymPEP-0001.md Outdated Show resolved Hide resolved
@gutow
Copy link

gutow commented Jan 28, 2021

Another thing that's missing is a discussion on what sorts of changes deserve a SymPEP. The reason is I'm not really sure about what that should be.

I think any change or addition to the behavior of SymPy that cannot be implemented in a single clearly defined step probably needs a SymPEP. I think that smaller new features or changes that do not impact the use of other parts of SymPy do not need a SymPEP. However, the inclusion of a larger package in the SymPy codebase should have a SymPEP even if the package could be removed without impacting the functioning of the remainder of SymPy.
To get things started here's some proposed language:

When do you need to develop a SymPEP?

  • If you are proposing a significant change or addition to SymPy a SymPEP should be developed to provide a roadmap for the alterations. This is true even if you are proposing to add a module to SymPy that only extends the existing SymPy codebase and could be removed without impacting any other part of SymPy. If the module is that independent, the SymPEP should address why the module should not be maintained as a separate Python module.
  • Long term SymPy policies and goals should also be discussed and developed as SymPEPs. Statements of these policies and goals should be incorporated into the SymPy documentation, as determined in the associated SymPEP.

@gutow
Copy link

gutow commented Jan 28, 2021

I like the template and will try it on with the SymPEP I started for the Equation class. I will provide feed back on what I learn.

@asmeurer
Copy link
Member Author

However, the inclusion of a larger package in the SymPy codebase should have a SymPEP even if the package could be removed without impacting the functioning of the remainder of SymPy.
To get things started here's some proposed language:

Yes, one of the first SymPEPs I want to write after this gets accepted is a SymPEP about the inclusion of new hard dependencies. One of the requirements there will be that each one should have a SymPEP.

If you are proposing a significant change or addition to SymPy a SymPEP should be developed to provide a roadmap for the alterations. This is true even if you are proposing to add a module to SymPy that only extends the existing SymPy codebase and could be removed without impacting any other part of SymPy. If the module is that independent, the SymPEP should address why the module should not be maintained as a separate Python module.

I'm not sure if every major feature should require a SymPEP, though. We're not like core Python where we need to be very stable and avoid adding too many new things. For me, it's more about if a change would benefit from the SymPEP process or not. If something really needs to be talked through before implementing it, that is something that needs a SymPEP. But if it's not really controversial or difficult, I don't think it necessarily does.

@gutow
Copy link

gutow commented Jan 29, 2021

My impressions after trying to fold the proposal for the Equation class into the template:

  1. It generally works pretty well for a proposal for code implementation/changes.
  2. I felt that the Usage and Impact and Detailed Description sections ended up being a little redundant. It might be worth trying to think of a more general single category that would contain the information of both these sections and be a better fit for the "informational" and "process" tracks.

To see how things fit see #1.

I'm not sure if every major feature should require a SymPEP, though. We're not like core Python where we need to be very stable and avoid adding too many new things. For me, it's more about if a change would benefit from the SymPEP process or not. If something really needs to be talked through before implementing it, that is something that needs a SymPEP. But if it's not really controversial or difficult, I don't think it necessarily does.

I then suggest using almost exactly what you said:

  • Not all significant changes or additions to SymPy require a SymPEP. If the change would benefit from extended discussion or needs a roadmap for implementation, a SymPEP should be considered. If unsure, consult with the
    community on the currently active discussion forum.
  • Long term SymPy policies and goals should also be discussed and developed as SymPEPs. Statements of these policies and goals should be incorporated into the SymPy documentation, as determined in the associated SymPEP.

@moorepants moorepants added 2019 and removed 2019 labels Jan 29, 2021
@naveensaigit
Copy link

One thing that I noticed is that the default branch for this repo is main. It might be better to make master the default since most of the users familiar with SymPy would be used to master. A change later on would probably not be good.

@asmeurer
Copy link
Member Author

GitHub is defaulting new repos to main instead of master now. I didn't bother changing it.

@gutow
Copy link

gutow commented Jan 29, 2021

  • Not all significant changes or additions to SymPy require a SymPEP. If the change would benefit from extended discussion or needs a roadmap for implementation, a SymPEP should be considered. If unsure, consult with the
    community on the currently active discussion forum.

  • Long term SymPy policies and goals should also be discussed and developed as SymPEPs. Statements of these policies and goals should be incorporated into the SymPy documentation, as determined in the associated SymPEP.

A somewhat edited suggestion for When do you need to develop a SymPEP?:

  • Not all significant changes or additions to SymPy require a SymPEP. However, if the change would benefit from extended discussion or needs a roadmap for implementation, a SymPEP should be considered. If unsure, consult with the community on the currently active discussion forum.
  • A significant change to SymPy behavior or the inclusion of a large module (even when it does not require code changes to the existing SymPy codebase or change the behavior of the remainder of SymPy) should probably have a SymPEP that justifies why it belongs in SymPy.
  • Long term SymPy policies and goals should also be discussed and developed as SymPEPs. Statements of these policies and goals should be incorporated into the SymPy documentation, as determined in the associated SymPEP.

@gutow
Copy link

gutow commented Jan 29, 2021

Does the SymPEP need a "What documentation needs to be developed in addition to the doc-strings?" section?

@asmeurer
Copy link
Member Author

The SymPEPs are about the process of how the specifics of those functionalities will be implemented. But the final result probably won't affect how it would be documented. Every library function should have a docstring with examples. Top level features should also ideally be discussed in high level guides (i.e., standalone RST documents in the Sphinx directory). This doesn't really change across features, so I would expect most SymPEPs wouldn't really have much unique to say about documentation.

If there's ambiguity about it, we could have a process SymPEP about what features require documentation (IMO, all of them). As we start to look at improving our documentation in more ways, especially expanding beyond just API documentation, that may be something useful to have.

@asmeurer
Copy link
Member Author

I included Informational as a category, copied from the PEP and NEP templates. But I'm unsure if it really belongs. It would seem that such things are better suited for the actual SymPy documentation. If you look at informational PEPs, they are things like the Zen of Python (PEP 20) or Python release schedules (I'm not sure why these are not Process). Confusingly, PEP 8 is considered Process, although I would expect it to be "informational". Their main purpose for Python seems to be to separate out things that are "required", in some sense, and things that the community is "free to ignore". So I propose removing this category. I think we really only need SymPEPs for two purposes:

  • To discuss and draft the design for major changes.
  • To formalize certain governance and process related things (like the deprecation policy and the policy for adding new dependencies).

@oscarbenjamin
Copy link
Contributor

Perhaps an "informational" SymPEP could be one that recommends using import sympy as sym and explains why that is better than the alternatives.

@asmeurer
Copy link
Member Author

asmeurer commented Feb 1, 2021

Perhaps an "informational" SymPEP could be one that recommends using import sympy as sym and explains why that is better than the alternatives.

Yes, that's what I was initially thinking. But now I'm not sure. It seems to me that that sort of thing should just go in the SymPy documentation. The other types of SymPEPs are partly targeted at users, so that they are aware of changes and can comment on them, but they are primarily targeted at people developing on SymPy itself. The informational SymPEPs break from this a bit.

With that being said, I don't feel strongly about it either way.

SymPEP-0001.md Outdated Show resolved Hide resolved
SymPEP-0001.md Outdated Show resolved Hide resolved
SymPEP-0001.md Outdated Show resolved Hide resolved
@moorepants
Copy link
Member

I think this is fine to merge, in general, especially if it is always considered "active" and we can update the process as we learn from using it. The only thing that would be nice to address is to be more explicit about what consensus means for SymPy. I think it means that there should be no -1 to the proposals from any person (sympy core developer or not) and that it is the job of the proposer to edit the proposal until people retract their -1. Is that the expected process? What if we have a very stubborn -1 giver? Are there contingencies?

@asmeurer
Copy link
Member Author

That gets into questions of governance, which might need their own SymPEPs.

@moorepants
Copy link
Member

So how will we accept PEPs that follow this one, or even merge this one, if there is no definition of consensus?

@Sc0rpi0n101
Copy link
Member

IMO having no opposition(-1) to a SymPEP from the whole community might be too much of a burden on the Proposer. It feels like an unreasonable expectation.

Since SymPEPs would generally describe significant features and processes for SymPy, people might have reservations about the SymPEP for their own opinions, and preferences.

“If there are no substantive objections within 7
  days from this email, then the SymPEP will be accepted; see SymPEP 1 for
  more details.”

This statement from line 184 seems like a better solution to the consensus problem, but I'm not sure what would be categorized as "substantive objections"

SymPEP-0001.md Outdated Show resolved Hide resolved
@oscarbenjamin
Copy link
Contributor

Looks good to me.

SymPEP-0001.md Outdated Show resolved Hide resolved
SymPEP-0001.md Outdated Show resolved Hide resolved
@asmeurer
Copy link
Member Author

asmeurer commented Aug 31, 2023

I know I'm the one who wrote this all in the first place, but we should see if we can't reduce the actual number of steps/process required here, or else no one will actually do it. e.g., can we trim down sections from the template, or remove steps from the requirements. It's easy to start getting super formal when building something like this, but it's actually better to only add formality when it's actually needed, especially considering that basically any process is already way more formal than what we've already been doing.

@moorepants
Copy link
Member

Do you have any specific suggestions on what to remove? I just read the steps again and the only thing I can imagine removing is how many times we ask the author to write mailing list posts (3 at the moment).

@moorepants
Copy link
Member

I guess a simpler method could be: 1) fill out the template and complete your draft, 2) announce on mailing list, 3) facilitate discussion to consensus, 3) accept, reject, or defer. But my worry with it not being more formal, is that it is then just the same process as a pull request is now. And it is clear we struggle to make large decisions that may cause large change. Things just stagnate without some process to fall back on.

@moorepants
Copy link
Member

One more thing, just because we merge this process and give it a try, doesn't mean we can't change the process at a later date.

SymPEP-0001.md Outdated

## Abstract

SymPEPs are a formal process whereby important changes are proposed and
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
SymPEPs are a formal process whereby important changes are proposed and
SymPEPs are a formal process whereby important controversial changes are proposed and

I just read this "Matplotlib Enhancement Proposals (MEP), inspired by cpython's PEP's but less formal, are design documents for large or controversial changes to Matplotilb." and liked the word "controversial". This may help people defer to a normal PR or mailing list discussion before coming to make a SYMPEP.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is the most significant point that's missing here, and it really deserves more text than just a single word. It isn't clear just what sorts of changes require a SymPEP and which changes can just be an issue or PR. I really think the default should be to not have a SymPEP, at least for codebase changes (process/governance changes are different). This isn't CPython. We don't need a proposal just to add or change anything.

Actually I'm not sure "controversial" is the right word to use here. A very big change may be uncontroversial, at least in principle, but still require a SymPEP because it requires a lot of coordination, or because we need to get everyone on the same page with the details.

Of course, controversial things should be discussed as well, but I'm a little unclear what that means. If something is controversial, shouldn't it just be rejected, because that's by definition a lack of community consensus.

@oscarbenjamin
Copy link
Contributor

Do you have any specific suggestions on what to remove? I just read the steps again and the only thing I can imagine removing is how many times we ask the author to write mailing list posts (3 at the moment).

Maybe just soften the wording to describe the process as more "semi-formal". I think that writing 3 posts to the mailing list is fine although it could be reduced. The posts to the mailing lists for announcing a proposal and for announcing a decision on the proposal are the only parts that I would definitely want to require if we are considering anything as a hard requirement. The most important thing is just that it should be clear to anyone who might be interested what forum they should be watching to know about these things and potentially want to join the discussion.

Otherwise where it says e.g. "a SymPEP should have these sections" could just be changed to "a SymPEP should usually have some sections like these example headings" etc.

@moorepants
Copy link
Member

I'm struggling to find the line you are referring to:

$ grep -r "SymPEP should"
SymPEP-0001.md:implementation, a SymPEP should be considered. If unsure, consult with the
SymPEP-0001.md:The author of a SymPEP should fork the repository and create a draft pull
SymPEP-0001.md:assigned to encourage discussion. Discussion on the SymPEP should primarily
SymPEP-0001.md:Eventually, after discussion, there may be a consensus that the SymPEP should
SymPEP-template.md:Any pull requests or development branches containing work on this SymPEP should
SymPEP-template.md:threads relating to a SymPEP should be listed and hyperlinked here.

@oscarbenjamin
Copy link
Contributor

I'm struggling to find the line you are referring to:

It wasn't a literal reference. Rather I was referring to the general tone.

Looking through though I don't think that the tone is very formal. Just the opening line says "SymPEPs are a formal process ..." which perhaps could say "SymPEPs are a semi-formal process ..." instead if we want to be clear that it is not completely formal. Also many uses of "should" could be changed to e.g. "could" like

A SymPEP might include an "Alternatives" section in which other solutions to the same problems could be discussed.

rather than

If there were any alternative solutions to solving the same problem, they should be discussed here, along with a justification for the chosen approach.

I don't think it matters that much but I guess if we want someone who is not a "core developer" to propose a SymPEP then the document should convey the intended tone.


**Author** list of authors' real names and optionally, email addresses
**Status** Draft | Active | Accepted | Deferred | Rejected | Withdrawn | Final | Superseded
**Type** Standards Track | Informational | Process
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't match the types listed in SymPEP 0001.

@asmeurer
Copy link
Member Author

asmeurer commented Sep 1, 2023

It's telling to look at the SymPEPs that have already been proposed and how much they are following this proposed process. For example, this one doesn't have all the sections in the template #3. Should that be a sign that the template here has too many sections, or that that particular SymPEP should be expanded?

not supported by GitHub, such as footnotes, should be avoided.
<!-- XXX: Perhaps we should abandon this and only require them to be
readable in some rendered format. That would allow us to use MathJAX, which
could be useful. -->
Copy link
Member Author

Choose a reason for hiding this comment

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

Math is supported in GitHub markdown now, so this can be removed. In fact, it might be worth pointing that out somewhere.

@moorepants
Copy link
Member

For example, this one doesn't have all the sections in the template #3. Should that be a sign that the template here has too many sections, or that that particular SymPEP should be expanded?

That one doesn't even look complete to me. It just ends, so I think that it is the latter: the sympep should be expanded.

@moorepants
Copy link
Member

Ok, I believe I have addressed all comments. It has been over two weeks since I sent the email to the mailing list with the 7 day deadline. I suggest we merge this.

@asmeurer
Copy link
Member Author

OK, should we set the status to "accepted" first?

Did we decide how merging should work in this repo? Should we merge things right away so that they are in the repo with draft status and a number? Or does merging mean accepting?

@moorepants
Copy link
Member

"Whenever a SymPEP moves from the Draft status to one of the other above
statuses, the header should be updated, and the corresponding pull request
should be merged. "

SymPEP-0001.md Outdated Show resolved Hide resolved
@moorepants
Copy link
Member

Status is changed to accepted.

@moorepants moorepants merged commit 1a4c0a2 into sympy:main Sep 18, 2023
@moorepants
Copy link
Member

First sympep is merged, thanks for all the help getting it through!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants