-
Notifications
You must be signed in to change notification settings - Fork 12
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 3: PyHC Python & Upstream Package Support Policy #29
base: main
Are you sure you want to change the base?
Conversation
For Python versions that age out of the proposed support window -- how firm is the expectation that package maintainers will drop support for the old Python release, in the case where there are no known incompatibilities? Could that take the form of documentation stating "Recommended Python version >= 3.X, but still works under Python 3.Y as of this writing", or would you want us to take more definitive action (bump python_requires to 3.X)? For example, if someone depends on a non-PyHC package that wants an older Python release, it could be a problem for them to upgrade Python to continue using PyHC packages. I've read some of the discussion around NEP 29, and I see the merit in the arguments about "who's going to take the plunge first and bump their package requirements?", and general community cohesion and predictability. Just wondering what the repercussions might be, in the event one of these messy real-world edge cases collides with what is otherwise sound policy. |
@jameswilburlewis that's an important question I'm wrestling with myself. I know some core packages like PlasmaPy and SunPy already go as far as bumping |
Just commenting and not formal review yet, since I think we're a bit more in a "discussion" phase than wordsmithing. As far as I can tell, PHEP 1 doesn't explicitly require the editor be distinct from the author, but I'd think it would generally be a good idea. I'd like to suggest expanding the scope to close #21: packages probably should be able to think about Python and other dependencies in the same context even if the principles are slightly different. I appreciate trying to keep scope reasonable but these seem interconnected to me. I really dislike the "everything not compulsory is forbidden" nature of SPEC 0. I don't think forcing our users to upgrade dependencies is a good idea. And given the difficulties with HelioCloud, we should probably err on being looser with "permitted" versions than tighter. This isn't something like Python 2 where a dedicated "kill the beast" plan was in order. So here's the sort of thing I'd like to see:
I can make edit suggestions to flow into Shawn's writing, but figured kicking the ideas around for a bit first would make sense. If any of these prove really controversial, we can just drop it out of the scope. tldr: support for a reasonable about of time. Be clear to your users. Don't leave your package uninstallable. |
SPEC 0 is the high level plan from the broader scientific python community, I don't see the need to be seperate from that push, we rely on all of their packages. Reducing the scope of what we need to support reduces the burden on all package maintainers within PyHC. We should also be telling users to create separate environments for each piece of work and that way can avoid pitfalls of updates breaking or messing with their current code or environment.
Typically for sunpy since we test with upstream on a cron job schedule, we don't need to worry about at least a smaller subset of package updates. We don't test the full suite so package updates that do break, will and do slip through, so we still have to patch and release at times for those. The main bottleneck is typically new python versions since we have a large dependency stack and we need to wait for those to explicitly support that python version but we try to push towards 3-6 months after release. Thankfully more core packages are testing sooner with python versions and their RCs so that timeframe is getting shorter.
I am hesitant to suggest max pinning of packages unless the package it self suggests it. In the numpy case due to their massive set of changes in the coming 2.0 release, it makes sense and it's pretty common in the sphinx world due how often they can break items in a release. But in my view, pinning either a max or a specific version should be discouraged unless you have really specific requirements in your package.
Ideally packages should add something like weekly or monthly cron job to test with "main" version of the core set of dependencies they use. Won't need to be all of them but it should at least cover the install dependencies. I don't think that adhoc testing is good enough for this, especially with how fast the python ecosystem moves. |
Thank you for the thoughtful comments, @jtniehof. And I appreciate the view from SunPy, @nabobalis! To what @jtniehof said, I definitely think it's best for PyHC's long-term success if we adopt the dependency version policy from SPEC 0. I was gonna push for it eventually, so I started questioning now whether it should be in scope for this PHEP. If people are game I'd like to include it here, but if it'll be a point of contention I'm more on the fence. I like your ideas though, especially having packages explicitly document their version policies. I plan to lead a discussion about this at Monday's telecon where hopefully I can start to get a sense of community consensus. If people seem onboard, I'd welcome and appreciate your edit suggestions. Let's see how people feel in the telecon then go from there? UPDATE: there ended up not being time to discuss this PHEP last telecon, so we'll have that discussion next telecon in two weeks instead. |
I think that's totally fair, in that case we should turn this PHEP in a more relaxed version: Try to support new Python releases within a time frame If the PHEP is just that, I guess that's more informationally than a requirement/standard? |
While this is a great point, I would say that the maintenance of a package is almost always unfunded. This is a problem with almost any library, it requires the deadicated time of a small group of maintainers or community contributions to keep a package ticking alone. I would personally argue if that you want to release and advertise your code/library/package that support from the authors/maintainers and them making sure the package is kept in a working (this meaning checking support for newer dependencies and Python versions, package metadata changes as the ecosystem moves etc) is the bare minimum required. This would be unfunded work normally, and I have little knowledge about what funding opportunities are available for this type of maintaince. |
Okay! I just pushed a change that I believe incorporates all the feedback from the comments here, while also expanding the scope of the PHEP to include the upstream package support policy from SPEC 0. The "drop" policy language has been softened to allow packages to continue supporting older versions if they choose to. The upstream packages touched by this new policy are clearly defined. Further recommendations have been added to the "Specification" section. If I missed something obvious please yell at me. Otherwise we're moving into more word-smithy territory now and I'd appreciate nit-picky wording comments and other such things. Also still seeking feedback about the |
I have a thought, which may or may not be in scope for this PHEP... The theme here seems to be promoting interoperability by setting expectations on how package maintainers will deal with upstream dependencies, specifically Python itself, and PyHC-core or scientific-python-core packages. But there are other considerations that come into play, especially for packages that supply binary wheels: OS versions and CPU architectures. New OS releases and new CPU architectures (e.g. Mac Intel -> Apple Silicon M1, M2, M3 etc) can both trigger a need to recompile non-Python library code. Should we have a policy on a timeline for supporting new OS releases or new CPU architectures with compatible wheels for PyHC packages? If I'm going to introduce a dependency on some other PyHC package for the sake of having a common way to handle coordinate systems, times, units, etc., I would hate to have a situation where installing my package on the hot new platform requires end users to compile their own C (or, God forbid, FORTRAN) libraries because binary wheels aren't available yet... |
Good point, @jameswilburlewis. We don't have any explicit statement requiring binary wheels right now and that feels out of scope for this discussion, but where we're talking about supporting new versions of Python in a timely manner, that seems to put issues like OS and arch in-scope. Maybe just wording that suggests support for the new must be at the same level as for the old--so if a package never does binary wheels (or conda, say) that's "okay" and users and potential downstream dependencies can make their decision, but if they've been releasing binary wheels people can reasonably rely on that in the future? |
I'm tempted to say that OS/architecture support is mostly out of scope here. The crux of this PHEP is really just "PyHC is jumping on the SPEC 0 bandwagon". Plus we already have PyHC standard 4: @jameswilburlewis @jtniehof Would it be sufficient to simply add a sentence like |
@sapols Sounds good to me! |
I might be even less specific: "packages should support new OS versions and CPU architectures to the same level as for previous environments". So whatever you were doing before, thou shalt do now. Up to the point of reason, of course...I don't deliver installer .exes for SpacePy anymore. |
Nice. With that change the paragraph becomes:
Is it clear what we mean by that without explicitly calling out binary wheels etc? I like how succinct it is, just wanna make sure it's clear too. |
Perhaps we could add a link to the corresponding page in the Python documentation or PyPA's packaging guide? |
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.
Please use one line per sentence it makes it so much easier to git diff and comment on.
pheps/phep-0003.md
Outdated
2. Support upstream Scientific Python packages for at least **24 months** (2 years) after their initial release. | ||
3. Adopt support for new versions of these dependencies within **6 months** of their release. | ||
|
||
The upstream Scientific Python packages are: `numpy, scipy, matplotlib, pandas, scikit-image, networkx, scikit-learn, xarray, ipython, zarr`. |
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.
Why this list of packages?
I know that they are the SP core packages, but why is that relevant to 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.
Simply because they're the packages controlled by SPEC 0. I wouldn't call it a comprehensive list of upstream packages PyHC cares about, but it's many of them and a great start. I was avoiding extending beyond the bounds of SPEC 0.
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 vote for dropping the list (see comment above).
pheps/phep-0003.md
Outdated
|
||
# Rationale | ||
<a name="rationale"></a> | ||
Following [SPEC 0](https://scientific-python.org/specs/spec-0000/)'s 24/36-month support timeline keeps PyHC in better sync with the broader Scientific Python community, maintaining compatibility with newer Python features and key upstream dependencies, while providing adequate time for package maintainers to adapt. Allowing 6 months to adopt new versions ensures packages stay current with development cycles while providing a reasonable timeframe for testing and integration. |
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 it should be worth adding to the rationale that reducing the number of versions supported by projects reduces the maintenance burden. It's almost impossible to test oldest supported versions of all packages in a pre-SPEC 0 land.
SunPy basically used to bump our minimum versions when our CI failed, since adopting NEP 29 / SPEC 0 we haven't had to do that at all. So we have a lot more confidence that our code actually works with a concrete set of minimum deps which are clear to everyone.
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 it should be worth adding to the rationale that reducing the number of versions supported by projects reduces the maintenance burden.
💯 Absolutely right! This is personally my main motivation for adopting SPEC 0.
It's almost impossible to test oldest supported versions of all packages in a pre-SPEC 0 land.
🦀 uv
has resolution strategies for --lowest
and --lowest-direct
which makes it pretty straightforward nowadays to test against the oldest supported versions of either all dependencies or direct dependencies. (We use --lowest-direct
for PlasmaPy because some dependencies don't list minimum required versions of their dependencies.)
SunPy basically used to bump our minimum versions when our CI failed, since adopting NEP 29 / SPEC 0 we haven't had to do that at all. So we have a lot more confidence that our code actually works with a concrete set of minimum deps which are clear to everyone.
Adopting NEP 29 / SPEC 0 has worked really well for PlasmaPy too in much the same way!
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 last sentence of the Motivation
section immediately preceding this Rationale
one is:
Additionally, limiting the scope of supported versions is an effective way for packages to limit maintenance burden while promoting interoperability.
We probably don't need to say that twice in back-to-back sections. @Cadair @namurphy would you prefer I move that sentence from Motivation
to Rationale
? Or leave it as-is?
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 point! Personally, I'll be happy with it so long as it's mentioned in at least one place in the 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.
My recent push kept it in the same place. @Cadair if you'd like me to move it under Rationale
instead I happily will.
Another thought from #31: Why don't we add a line or something here strengthening the maximum or hard pin requirement to say that "you MUST not require versions of any dependency older than 24 months?" This would go a long way to removing conflicts when trying to install all compliant packages in the same env? |
…rt; remove "indirectly" & "GitHub"; no pinning outdated deps
Okay I pushed another round of changes that capture all comments added since the last push:
I think this document is looking pretty strong now. I still want feedback on the |
pheps/phep-0003.md
Outdated
# Open Issues | ||
<a name="open-issues"></a> | ||
1. What should go in the "How to Teach This" section? Should we expand on the ideas already there or take it a different direction? | ||
Or leave it if it is sufficient 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.
Note that in the absence of feedback I'd now consider "leave it" the default option 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.
Other than my one suggestion, I think that's sufficient.
#16 has a new suggested standard (12 in there) which is worth looking at in this context. Also, I suggest we explicitly note that this replaces standard 11: Such a note probably belongs in "how to teach this" ("This section should also document any changes the PHEP makes relative to a PHEP it replaces or some other widely-used standard or reference"). I don't know if we want to use the "Replaces:" header for standards which are replaced. |
We actually already explicitly note this replaces standard 11 throughout, in the Abstract, Motivation, and Specification. |
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.
Really good--very clear. I have several suggestions for consideration but I don't think they're "fatal". So leaving this as a "comment" for now and potential approval pending those conversations.
|
||
# Abstract | ||
<a name="abstract"></a> | ||
This PHEP recommends that all projects across the PyHC ecosystem adopt a common time-based policy for support of dependencies, inspired by [SPEC 0](https://scientific-python.org/specs/spec-0000/). |
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 "recommend" the right word? This gets into the question of applicability of PHEPs which I think we're still feeling out as a community, but I'd suggest stronger wording:
"This PHEP establishes a common time-based policy for support..."
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.
This is something Julie and I should discuss but I suspect we'll end up leaving the "recommend" wording. This PHEP was originally written as more of a "must" but the first major pushback I got in the earlier comments were people requesting I soften the policy; hence why We decided this policy has to be a "should" not a "must."
is the second bullet under the Resolved questions and comments
section of this PR's description. It also more closely follows SPEC 0's wording which uses "recommend."
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'll add that even if I started this first sentence with "This PHEP establishes a common time-based policy for support...", the word "recommends" occurs immediately in the next line. So I'd have to change that wording and all subsequently-related statements to match, which again goes against what we'd previously decided.
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.
Not a hill I intend to die on. But maybe worth putting explicitly in the rejected ideas? E.g. "it was considered making this a requirement rather than a recommendation but the community argued against because xyz"?
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.
Done.
pheps/phep-0003.md
Outdated
2. Support upstream Scientific Python packages for at least **24 months** (2 years) after their initial release. | ||
3. Adopt support for new versions of these dependencies within **6 months** of their release. | ||
|
||
The upstream Scientific Python packages are: `numpy, scipy, matplotlib, pandas, scikit-image, networkx, scikit-learn, xarray, ipython, zarr`. |
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 want to list these, or simply reference SPEC0 and leave it at that? Implicitly then as SPEC0 updates the goal of this policy would update.
I also do like wording like "other dependencies which follow similar versioning schemes should be similarly supported."
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 believe there's merit in being clear about which packages this policy applies to by explicitly listing them. And they do say that their list of core projects will not change rapidly. But I do like the idea of this list implicitly updating if the core Scientific Python packages ever do change, so here's what I'll do:
- Say "upstream core Scientific Python packages"
- In the Abstract, change it to:
At the time of writing, the upstream [core Scientific Python packages](https://scientific-python.org/specs/core-projects/) are: numpy, scipy, matplotlib, pandas, scikit-image, networkx, scikit-learn, xarray, ipython, zarr.
- In the Specification, clarify that by saying:
At the time of writing, the upstream [core Scientific Python packages](https://scientific-python.org/specs/core-projects/) are: numpy, scipy, matplotlib, pandas, scikit-image, networkx, scikit-learn, xarray, ipython, zarr. If their core packages are updated, this policy applies to the updated list instead.
Additionally, while your wording about other dependencies sounds good, I don't want to be vague and introduce uncertainty about which packages this policy applies to. We're simply adopting SPEC 0 here, and SPEC 0 only applies to their core packages, so our policy should do the same.
pheps/phep-0003.md
Outdated
2. Support upstream Scientific Python packages for at least **24 months** (2 years) after their initial release. | ||
3. Adopt support for new versions of these dependencies within **6 months** of their release. | ||
|
||
The upstream Scientific Python packages are: `numpy, scipy, matplotlib, pandas, scikit-image, networkx, scikit-learn, xarray, ipython, zarr`. |
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 vote for dropping the list (see comment above).
<a name="specification"></a> | ||
This PHEP refers to feature releases of dependencies (e.g., Python 3.12.0, NumPy 2.0.0; not Python 3.12.1, NumPy 2.0.1). | ||
|
||
This PHEP adopts Scientific Python's [SPEC 0](https://scientific-python.org/specs/spec-0000/) and specifies that all PyHC packages should: |
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.
Rather than "specifies that all PyHC packages should" I'd say "specifies that packages must". I.e. a package that is compliant with this PHEP must support that much. The question of the consequences of noncompliance feels more appropriately dealt with elsewhere (e.g. PHEP4 which I haven't looked at yet :) )
I also think involvement in PyHC is sort of implied in the context.
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.
This goes back to my reply above about how we previously decided this policy has to be a "should" not a "must." With regard to PHEP 4, rather than thinking about it as "consequences of noncompliance" I'm thinking about it like "if you follow PHEP 3's recommendation then you are compliant for PHEP 4's sake."
Point taken about PyHC involvement being implied, but saying "PyHC" only adds one word and I like the clarity :)
pheps/phep-0003.md
Outdated
- A new web page on the PyHC website will detail the support policy and include a graphical timeline of the schedule (similar to the Gantt chart above). | ||
- Automated email reminders will be sent via the PyHC mailing list quarterly and near important drop/support dates to remind package maintainers of the schedule. |
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 detail" and "will be sent" are somewhat passive. Suggest this be an explicit burden on the tech lead..."the PyHC tech lead will regularly update a graphical timeline and set up automated email reminders....". I do like this!
I apologize for missing that standard 11 is mentioned all over the place :) It might be worth mentioning here that since this completely supersedes standard 11, people should probably start from scratch in making their plans, rather than using standard 11 compliance as a basis.
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's definitely gonna be my burden so I agree it'd be good to clarify this is the Tech Lead's job! I'll reword.
One notable example is PlasmaPy which currently [documents their SPEC 0-based policy](https://docs.plasmapy.org/en/stable/contributing/coding_guide.html#python-and-dependency-version-support) and even mentions it in comments inside their [pyproject.toml](https://github.com/PlasmaPy/PlasmaPy/blob/main/pyproject.toml) file. | ||
|
||
## Code to generate support and drop schedules: | ||
```python |
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 it be explicitly noted that this code is the source of the included Gantt chart? Does it make more sense to put it elsewhere (in this repository or otherwise) and link it from here, as something that is live updated as necessary without having to update the 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.
You got it. I'll add "The following code can be used to generate support and drop schedules, including the Gantt chart above.
" between the header and the code.
I considered putting the code elsewhere, like in a separate file and linking to it, but decided not to for two main reasons: (1) SPEC 0 included their code in-line inside the SPEC and I liked that. (2) I honestly do not intend to formally maintain this code. Sure the dates etc will eventually become obsolete after enough time passes, but ain't nobody got time to remember to update dates in an obscure script! It's valid now and a good enough starting point for anyone who wants to use it in the future (likely only me tbh).
pheps/phep-0003.md
Outdated
# Open Issues | ||
<a name="open-issues"></a> | ||
1. What should go in the "How to Teach This" section? Should we expand on the ideas already there or take it a different direction? | ||
Or leave it if it is sufficient 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.
Other than my one suggestion, I think that's sufficient.
suggest change: support these packages IF they are a dependency of the package |
Related note: I raised astral-sh/uv#7515 as a feature request to uv to automate updating of the minimum allowed versions of packages in |
This new policy replaces the current standard [#11](https://github.com/heliophysicsPy/standards/blob/main/standards.md#standards) in the PyHC standards document with the following new text: | ||
|
||
> **11. Python and Upstream Package Support:** All packages should support minor Python versions released within the last 36 months (3 years) and upstream core Scientific Python packages released within the last 24 months (2 years). | ||
Additionally, packages should support new versions within 6 months of their release (see [PHEP 3](https://github.com/heliophysicsPy/standards/pull/29)). |
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 need this? This PHEP replaces standard 11, it doesn't create new text for it. 11 would just go away and people just need to comply with PHEP3 instead of Standard 11.
This is an excellent summary text for a potential summary document, though.
(Sorry I missed this earlier.)
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 gonna say yes we need this. For now, the old standards doc is still PyHC's official standards and I will replace the text of standard 11 in that doc with what's written 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.
Let's discuss in the morning how we want to handle this; fortunately that's before the PHEP3 vote :)
Overview
This is the initial draft of PHEP 3, which proposes adopting a Python version & upstream package support policy for the PyHC ecosystem, inspired by SPEC 0. The goal is to standardize the support duration for Python versions and popular packages across all PyHC packages, ensuring a balance between stability and the incorporation of new features.
Specifically, this PHEP recommend that projects:
The upstream core Scientific Python packages are:
numpy, scipy, matplotlib, pandas, scikit-image, networkx, scikit-learn, xarray, ipython, zarr
.This policy aims to replace the current standard #11, which mandates only Python 3 support, with a more structured timeline that supports consistent and predictable maintenance across the community.
This closes #21.
This closes #20.
Renders
Rendered current text of the PHEP
Render of PHEP before scope was expanded to include upstream packages
Inspiration
This PHEP was inspired by the Python version support policies listed in:
Open questions and comments
Resolved questions and comments
python_requires
to3.X
) in favor of softer language that allows packages to support older dependencies for longer if they want.