Developer infrastructure: mark code with level of user-exposure

[rileyjmurray]

pyGSTi is both research software and a tool that's used in "production" QCVV. Its research flavor makes it impractical for us to spend a lot of effort differentiating between a public and a private API. At the same, its role in production code means we need to be judicious about changing pyGSTi in a way that would break users' workflows. These factors combined make it hard for newcomers (like myself) to contribute to pyGSTi. Here's a proposal to help get new contributors un-stuck.

Proposed policy

I suggest the following taxonomy for marking a piece of code's level of "user exposure."

• High exposure: API changes are likely to break workflows for some users, including "casual" users.
• Low exposure: API changes are unlikely to affect casual users of pyGSTi but might affect some of pyGSTI's advanced users.
• Minimal exposure: while it's conceivable that API changes could break user code, the possibility of this is sufficiently remote that we can change the API at will.

I think everyone is on board with a taxonomy along those lines. However, we didn't discuss how these labels would be used to affect pyGSTi's development. I suggest we use it in the following way.

• API changes to high-exposure code require the support of all pyGSTi maintainers. (EDIT: They'll also have to go through a formal deprecation process.)
• API changes to low-exposure code require the support of a majority of pyGSTI's maintainers.
• API changes to minimal-exposure code can be made with the support of any single maintainer.

For now, I'd take "pyGSTi maintainer" to mean @sserita, @coreyostrove, or @enielse.

Note: In the call, we joked a bit about marking certain code as "godforsaken," to signify that we'd like to change it substantially, but we don't quite know how or when to do so. While I think that would be helpful, I don't think we need to figure that out now.

Proposed adoption plan

Over the coming months, the pyGSTi maintainers mark all "high exposure" code. Low- and minimal-exposure code is marked along the way, to the extent possible.

The adoption process will effectively complete when we're comfortable with implicitly designating unlabeled code as "minimal exposure." API changes to any such code only require the approval of one maintainer, but the approving maintainer has a responsibility to think about whether or not the code actually has minimal user exposure.

Request to pyGSTi developers

Comment below indicating if you agree with this general direction, or if you'd like changes. Once we converge on what we want to do, we can figure out how exactly we'll do it.

[rileyjmurray]

Here are some notes from our October 17 meeting.

Everyone's on board with the proposal I made above. (Modulo one edit I had to make.)

We'll implement tags concretely with one-line comments before any def to indicate high, low, or minimal exposure.

It's important to note that these markings are kind of our guess at what the community is using. Their shelf-life is limited.

Adding these kinds of tags is the first phase of a three-phase effort. The second phase is to replace comments with things that are more permanent or functional (like prepending an underscore to the names of minimal-exposure functions). The third phase will be to public API. We have no goals for when we start phase two.

We should keep the community appraised of this effort as we go along. We can have an "API Stability" page of the pyGSTi website that explains the purpose of the tags, solicits user input, and gives advance notice on bigger changes we might make.

CC @sserita, @enielse, @coreyostrove

[coreyostrove]

Excellent work and summary, @rileyjmurray! I am onboard with all of the above.

One minor addendum to the summary. We also discussed integrating the maintenance of this API tagging into our standard PR workflow as part of an update to our contributor guidelines (much like we are effectively handling unit testing additions and documentation presently).

[sserita]

Sounds good to me. Thanks @rileyjmurray for the great work and I agree with @coreyostrove's addendum - really this will also come down to us as reviewers to ensure that any changes to exposure-levels of the code and updating the tags accordingly. This will be most relevant in the second stage where we are working to shift code up and down levels as we work towards some more permanent organization structure I think.

[coreyostrove]

Quick heads up for anyone working on the branch related this this github issue that I temporarily disabled the on-push server-side unit tests for this branch. We'll be making a lot of comment-only pushes as part of these updates, so I figured it wasn't a great use of github runner allocations to be testing against it constantly.

[rileyjmurray]

Good catch!

…

On Tue, Nov 28, 2023 at 9:53 PM coreyostrove ***@***.***> wrote: Quick heads up for anyone working on the branch related this this github issue that I temporarily disabled the on-push server-side unit tests for this branch. We'll be making a lot of comment-only pushes as part of these updates, so I figured it wasn't a great use of github runner allocations to be testing against it constantly. — Reply to this email directly, view it on GitHub <#354 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACRLIFBHNDITREEAZASMADDYG2PQ3AVCNFSM6AAAAAA54RXZRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZRGEZTCMBWGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[coreyostrove]

Hey @kmrudin and @jordanh6, @rileyjmurray and I did some user-exposure tagging (see above on what that means) of the various algorithms submodules. This included our best guesses for submodule -level visibility for some code that you both own (RB-stuff for @jordanh6 and RPE-stuff for @kmrudin). Could you take a look at the changes in this commit 6565ad7 and if you disagree with our assessment let us know?

[jordanh6]

The RB parts look reasonable to me.