Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
PHEP 4: PyHC Package Tiering #31
base: main
Are you sure you want to change the base?
PHEP 4: PyHC Package Tiering #31
Changes from 6 commits
beb0c9e
4827407
10b6add
0ebfa4b
5f6a9a6
60620dd
c44670c
fa310db
367748b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHEP-1 specifices "PHEPs must not use GitHub extensions" (We discussed this a little bit). An HTML block is a potential alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH, I had to look up what those differences were (I just googled how to make a table in markdown and did it this way). What exactly needs to change? @jtniehof
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think requiring a html table is a good idea. it would make the source hard to read and almost impossible to comment on inline for review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommonMark doesn't support tables; they are a GitHub extension to the standard.
Discussions on that in the PHEP1 thread: 1, 2 ... this was not extensively discussed, which I took as "accepted right off" but might have been "lost in the noise".
Practically speaking, I checked and reText doesn't support tables. pandoc with
-f commonmark
doesn't. With-f markdown
or-f gfm
it does. (Although withgfm
the table cells don't wrap, and it runs off the page). vscodium previews it fine. (EDIT: Google Docs imports it okay, too.)The options as I see (no particular order) are:
I don't know if we want to take this to a discussion outside of this PHEP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtniehof This might be a good question to bring up at the next telecon? My gut reaction is to either do steps 3 or 4. Presenting in anything other than a table seems less intuitive/easy to digest, and I'm hearing that using an HTML table is a bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussion in #38
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how I feel about relegating GPL packages to the third tier when they could be excellent in all other regards. I am aware of why we don't recommend copyleft licenses, but sometimes it can't be helped. GPL (etc) are very good licenses which people can have legitimate reasons for using. If we really had a package which that was it's only "bad" grade would we really want to relegate it to effectively the lowest tier when it's a real possibility the authors would be unable to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to changing this. In your mind, what would the levels be @Cadair ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might lump GPL and NOSA in the same category...agreed that making GPL "worse" than NOSA seems a bit much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be fine with Jon's idea on this. So, NOSA + copyleft licenses would be bronze tier? Silver and gold would both require a recommended license (not to include those)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, these suggestions work for me, I'll update to allowing them at Silver, but not Gold, tier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not convinced that we should be listing packages without a license at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use case that comes to mind for this would be code associated with a publication, particularly in cases where the publisher demands a separate DOI for the code. Then again, should those be included in PyHC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points... I feel like this is a topic to talk with the community as a whole about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"No license" would mean nobody actually has the right to distribute or use the code. Suggest "non open source license" as the lowest tier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm torn here, and this is related to another discussion above on the licenses. I agree that this would greatly benefit community discussion. Do we already have packages that have no license?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what I'll do for now is put "non open source license" for copper tier. But I have a list of questions for the community to bring up at the next telecon (we're discussing PHEPs then).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current/(new?) license row doesn't look right. The gold level should not include GPL or NOSA licenses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I accidentally did an incorrect paste when modifying things. Fixing now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend including links to pages that explain how to do each of these things, to make it easier for new people to fulfill each requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I'll add that/include in the "How to Teach This" section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be standards, or PHEPs, or "standards and their replacements?"
As I noted in #33, #34, #35 it would be nice to replace our standards columns on the package page with PHEP compliance. More below.
Then instead of standards grades it would be more "compliance with standards-track PHEPs" and something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the table approach so the requirements for each level are more specific and clearly laid out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am understanding correctly, in the end this would still be a kind of stoplight system like what we have now, but pointing to compliance to... PHEPs? I didn't see your issues before submitting my code here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, stoplight system but the columns are PHEPs instead of categories of standards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to go along with that. But, does that also mean this PHEP has to sit in limbo until those other complimentary PHEPs get passed? @jtniehof
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I like leaving the "many should requirements" in the Gold-level packages. We really should have only the cream of the crop and those putting in the effort to fully comply requiring our highest tier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, put this in a related but different discussion thread, so now subjecting you to copy/paste:
I wouldn't think we need to hold this up until all standards PHEPs are done. If we have something like "when new standards-track PHEPs are approved packages have 6 (12?) months to self-evaluate and update their tiers", then we could in theory approve this with no new standards lined up. There's going to be a transition period regardless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this anything unique to PyHC, or just being in a pyOpenSci review? If the latter, can this link the appropriate pyOpenSci page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be specifically for the PyHC-pyOpenSci pairing review process (checking against both pyOpenSci reqs + PyHC-specific reqs). No link for that, yet. That's part of why I'm trying to get the community to chat about what we'd need to define "yes, fits in with the PyHC" during the pyOpenSci process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe explicitly say "to be defined in the future" or something? It feels a bit weird to be approving a standard that requires something that's not yet in place but I understand we can't do everything at once. Or this could be made more vague of "future collaborations" or something and the PHEP defining the pyOpenSci process would say "modifies PHEP 4 by adding...."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That kind of modification plan could work nicely. In that pathway, this PHEP would become the skeleton (most of) the other PHEPs would map to using a stoplight or yes/no system as appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some items, we can flesh it out here and not wait for another PHEP. Others will need this approach or something similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'm going to modify the wording a bit here based on this feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this PHEP 3 compliant, or is this installable within our reference environment? Suggest getting rid of this and treating PHEP 3 however we treat any other standard/PHEP. And we would likely need another PHEP defining that reference environment....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Namely just a way of saying "do you play nicely with our environment". I don't think it'd be appropriate to have a Gold-level package that cannot install into the PyHC environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but I don't think PHEP3 compliance will automatically make things installable within the environment. So which is the requirement: following PHEP3, or installable in the environment? Or two separate requirements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like these should be two separate requirements. But maybe we start with the 'installable' comment and work other PHEPs later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need TBDs resolved before voting. Can we link the HSSI standards so we know what we're signing up for? Also can probably remove "new"; it won't be new forever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HSSI standards are not yet fully developed, is the thing. But perhaps they could be very soon. @rebeccaringuette ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure on removing the word new though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely. We should have something worthy of discussion for the fall PyHC meeting to get feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest either pairing approval of this PHEP with development of the HSSI standards or, like with pyOpenSci, do an update PHEP later once they're developed. (Perfectly fine for this to explicitly say "standards to be defined in later PHEPs such as pyOpenSci and HSSI").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second option is likely needed for the HSSI metadata requirements. I do intend to present a draft of those suggested requirements per package level at the fall meeting. There is a skeleton in a comment already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be treated like any other standard instead of making it a separate bullet here? I'm not sure about calling out NOSA. It is OSI-compliant but not FSF-compliant. There might be some issues if a package needs a NOSA dependency (or ships it alongside)--is that an immediate "can't be gold"? How is NASA doing at getting things relicensed? The CDF license is somewhat ambiguous re DFSG and I don't know if it's been FSF or OSI evaluated (I don't think it has).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The process is ...interesting at NASA to get things relicensed. At Goddard, the leadership knows the problem in detail and is having significant problems moving forward. Hopefully a leap-frog moment is on the horizon.... With that said, NOSA is worth calling out because it prohibits contribution except through a long registration process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was suggested by @rebeccaringuette to remove NOSA as a possibility due to the reasoning she gave here. It seems reasonable to me. Might even help NASA to improve that license (if that's a possibility) if a software community directly calls out not using it.
Perhaps this bullet could go away if we get standards PHEPs in to point to (where this NOSA issue could be put)? I do worry though about this taking a long time to enact though if it's dependent on other PHEPs all getting written, approved, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't think we need to hold this up until all standards PHEPs are done. If we have something like "when new standards-track PHEPs are approved packages have 6 (12?) months to self-evaluate and update their tiers", then we could in theory approve this with no new standards lined up. There's going to be a transition period regardless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend having these be something people can opt in or out of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't even think of that, that's a good idea. I'll modify to indicate that that would be an optional perk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend dropping this from the table completely. I am sure there will be lots of smaller things like this that projects will or wont get pulled into. This feels very frivolous to include in a very formal specification document.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cadair I disagree that it's frivolous. As PyHC-Chat improves, inclusion within it could be a carrot—not the only, or biggest, but still a carrot—for people to work to move tiers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a PyHC benefit or just part of submitting the correct HSSI metadata? Can PyHC put requiremnents on HSSI? Similar for pyOpenSci.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyHC would be requiring PyHC packages at a given level to contribute a given list of metadata fields. PyHC and HSSI are working together on what those lists are. Similarly, PyHC and pyOpenSci are working together to figure out what the requirements should be in that review. Likely needs a PHEP for the pyOpenSci.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, what @rebeccaringuette said.
Agreed on the PHEP for pyOpenSci. It'll need community discussion to come to terms with what reqs we want for a package being "PyHC-compliant" during a pyOpenSci review process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if something is otherwise compliant with pyOpenSci, but isn't doing something right on the PyHC side so it's silver, it won't be included in pyOpenSci? (HSSI inclusion is listed for all tiers so not as relevant).
It seems odd to say a benefit of a particular PyHC tier is being listed elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of the pyOpenSci review process will have items or characteristics that are specific to PyHC, similar to that custom pyOpenSci/SunPy review process in development (or now realized, not sure). The point of going through that review process is not to be listed elsewhere, but to decrease the effort needed for PyHC leadership to review those same details of all PyHC packages by hand.
Or did you mean something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm starting to be convinced that the pyOpenSci badge being included with a package indeed should be separate from this PHEP. I think I'm going to remove it, and it can be a separate thing added to a package (but the standard of Gold/Silver for pyOpenSci would still stand, just not then the benefit of the badge being part of this PHEP).
As for the HSSI, I think it still makes sense to include here for PyHC-specific packages, if nothing else than to really encourage packages to get the proper metadata in for a software search interface dedicated to Heliophysics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really have control over this? Can it be "Community recommended for conference travel funding?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, yes and yes. Personally, I would not want a lower level package even informally representing PyHC at a science conference. Preferably, those would be silver and gold.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtniehof yes. Basically because of what Rebecca said. I also wrote into the next PyHC leadership grant some funding to send package leads to conferences, so I want some kind of guidelines for how to pick said packages. I think it makes sense to have packages that have put in the work to develop in line with PyHC needs to be the ones representing us at meetings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Could this be "considered for PyHC travel funding assistance..."? Someone can put in their own TWSC proposal without this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah @jtniehof I see now where the confusion came in. I'll add in PyHC so it's clear what travel funding money pot we're referring to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth a forward reference to the implementation process..."are evaluated" by whom?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in, mention the TSC that reviews packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TSC = ???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rebeccaringuette Technical Steering Committee. Jon used it in a comment above so I just kept up with the acronym.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, or "evaluated (as described in 'Implementation Process' below)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need further definition with the PHEP, or is the promise of documentation sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A note that this will supersede the existing project submission process is probably helpful. Also potentially a list of differences:
Plus, of course, a commitment to update the submission process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted!