APE 21: APE on ending long term support releases + Updates to APE 2 to reflect new release cycle without LTS

[astrofrog]

This is a draft and not ready for review - it follows the related discussion on astropy-dev. If you are interested in joining as a co-author please ping me!

• [x] Rename file to APE_21.rst

EDIT: Close #20

[mhvk]

Had a quick look and liked this! Agreed that it will be important to be very clear about minor and major. After reading this, and my original proposed addendum to #20, I think the proposal is:

• Minor releases mostly should add things and can, if needed, deprecate things. Generally, existing code should continue to work (with possible filtering of warnings), so little if anything should be removed..
• Major releases are the same but can also remove deprecated features. So, existing code may need to be adjusted (but would have had ample warning).

Is this correct? Perhaps good to give examples. E.g., "existing code should continue to work" probably should not include that every repr remains the same, or every number exactly identical. But it would include not changing API.

[astrofrog]

@mhvk - I agree with your assessment, I think we basically have to decide on guidelines for those exceptions of things that can change in a minor release that might be considered as 'breaking'. Things that are borderline would include:

1. Changes in warning messages
2. Changes in exception messages or exception types
3. Removing functionality that was not really intended to ever be public and is probably not used (for example some random function deep in astropy.utils that is not technically private)
4. Fixing recent API 'bugs'/mistakes that would be easier fixed sooner rather than later if it can't be fixed via a regular deprecation

Anything else?

I think something related to this APE that we don't have to necessarily figure out here but that we could mention briefly is that if we are going to have rules like this we should also be clearer on what is public API - anything tab-discoverable? Anything in the API docs in docs.astropy.org? (those two don't necessarily match currently though I think I saw @nstarman mention something about public API in another PR). Defining what is public API could almost be a separate APE.

[mhvk]

I think all four are fine in minor releases -- certainly exception and warning messages (I think it is fine to break people's tests if those are so detailed they check messages, just not to break actual code!).

But very good point about public API. The most narrow (and probably best) definition is that it is only includes thinkgs exposed at the top level of astropy's submodules (with a few exceptions in utils like iers and masked). But you're right, this is probably another APE, with the main goal to actually document our current behaviour!

[pllim]

How many will cry if I change astropy.utils to astropy._utils in v6.0? 😝

[pllim]

Why are we avoiding double quotes? I thought English prefers double quotes and you only use single quotes if you have to quote inside text that is already in double quotes?

[astrofrog]

Happy to change to double quotes! Can we just run black on the APE? :stuck_out_tongue: Feel free to push a commit to fix this if you have time :smile:

[pllim]

@astrofrog , I took some liberties in 22c9073ebf4cbfe6cef21cb189fa5f4631eba2f1 , feel free to revert the changes you don't agree with. Thanks!

[nstarman]

I think something related to this APE that we don't have to necessarily figure out here but that we could mention briefly is that if we are going to have rules like this we should also be clearer on what is public API - anything tab-discoverable? Anything in the API docs in docs.astropy.org? (those two don't necessarily match currently though I think I saw @nstarman mention something about public API in another PR). Defining what is public API could almost be a separate APE.

I would propose we adopt the PEP8 standard (https://peps.python.org/pep-0008/#public-and-internal-interfaces) and the typing guidelines (https://github.com/python/typing/blob/master/docs/source/libraries.rst#library-interface-public-and-private-symbols). The latter offers some small refinements to the former, namely to ensure that analyzers, static and dynamic, can easily understand what is public and help users with better tab autocomplete / code snippets / generative code, etc. In short what we need to do is define __all__ in every module indicating what is public in that module. For the most part this is having a full __all__ at the top-level __init__.py of each sub-package and empty __all__ everywhere else. Technically, PEP8 suggests that we also rename modules to have a leading underscore.

[mhvk]

Hmm, those texts are helpful but not quite conclusive -- or at least I can read it in ways that I'm fairly sure are different from other's! Anyway, to me it sounds like if in a submodule's __init__ we do not import the actual subsubmodules (say, in coordinates.__init__, we import from representations, but not representations itself), then we basically are OK (especially if we also define an __init__.__all__). And that in units, .si is a public module since we import it in .__init__ (which may not be what we want, so we do need to define __all__ there).

I guess the only part to then clarify is that a subsubmodule should only define things in __all__ that are actually imported in the submodule proper.

p.s. Probably should move this to a different thread; sorry!

[pllim]

@astrofrog , is your plan to discuss this APE at the Coordination Meeting?

[astrofrog]

Yes but only if we can't agree on it sooner!

[saimn]

I like the proposal for several reasons:

• the meaning of version numbers should be easier to understand for users (in contrast to LTS where people may hesitate on which version to use)
• users are more likely to update to the latest minor version since it should not break their code
• for people that were using LTS, they can update to the latest minor version and get new features more rapidly

The difficult point is how to define an API break. Tom gave some examples in https://github.com/astropy/astropy-APEs/pull/82#issuecomment-1438161547, are changes to exception or warning messages API breaks ? IMO It's hard to define strict rules here, it depends on the impact of a change, if the code that is changed is widely used or not etc. So when reviewing PRs maintainers will need to think if something is likely to break other people's code.

There are cases where breaking the API is needed to fix a bug, depending on the impact some may be fixable in minor versions, other may have to wait for major versions with a deprecation path. I like pandas' note about this: https://pandas.pydata.org/docs/development/policies.html#policies-version

pandas will sometimes make behavior changing bug fixes, as part of minor or patch releases. Whether or not a change is a bug fix or an API-breaking change is a judgement call

(And I like the way they have written their policy, it's simple so easily understandable by users)

Also we probably need a way to track deprecations so we know what changes need to be done in major versions ? And I think the discussion about public API is not relevant here, it's an interesting one but it should be discussed somewhere else. Here the focus is on how to handle deprecations, and how to provide to users new versions with some guarantees of stability.

[mhvk]

Agreed that the pandas wording is very nice & clear. I suggest we use it! I think the main trick is in defining what API is. In my opinion, how something is typeset (repr) or worded is not included in the API (for a minor release), so the wording of an error message is not, but the structure/type/behaviour of an object or the exception type is part of the API.

[taldcroft]

so the wording of an error message is not [in the API]

The problem is that the wording of an error message is very commonly used in tests, and sometimes used in application code. So these kind of changes can certainly break downstream code, and it once again comes down to an assessment of impact.

The only way out of this I can think of is to have some policy for advertising API changes and getting a wider review. For instance, if a PR has a change that has the potential to break application code or break other package tests, then it could be brought to the attention of astropy-dev. E.g. an email with a subject like Astropy PR #yyy API change for review, with some text describing the potential impact. Allow a minimum 2-week period for review prior to accepting the PR. That extra layer of review would also serve as a reminder to developers that API changes should be avoided when possible.

The "potential for breakage" bar should be set pretty low since we aren't stopping any change from happening, but doing our best to ensure that the community is aware of a change in advance. Definitely warning and error message changes would apply.

[WilliamJamieson]

The only way out of this I can think of is to have some policy for advertising API changes and getting a wider review. For instance, if a PR has a change that has the potential to break application code or break other package tests, then it could be brought to the attention of astropy-dev. E.g. an email with a subject like Astropy PR #yyy API change for review, with some text describing the potential impact. Allow a minimum 2-week period for review prior to accepting the PR. That extra layer of review would also serve as a reminder to developers that API changes should be avoided when possible.

On top of this I think we should explicitly encourage maintainers of downstream packages to run their tests against the development version of astropy regularly (with some suggestion of what regularly means).

[mhvk]

Agreed that we should encourage people to test against astropy-dev.

I do want to push a bit more to keep the freedom to change formats or error messages in minor releases. If people rely on details like that (presumably mostly in doctests), then I think it is fair to ask them to adjust, just like we adjust when numpy changes its output format (and, yes, sometimes that is painful, but definitely OK for a minor release).

On error and warning messages specifically, in astropy we check those internally just to ensure we actually raise the right error, but really why would anyone rely on those outside of astropy? I don't think we have any place where we specifically look for the text of an external error or warning. And it does seem one should be free to correct or clarify exception texts, certainly in a minor release, arguably even in a bug-fix release.

Anyway, I really hope we can keep things more or less like they are for minor releases!

[WilliamJamieson]

On error and warning messages specifically, in astropy we check those internally just to ensure we actually raise the right error, but really why would anyone rely on those outside of astropy? I don't think we have any place where we specifically look for the text of an external error or warning. And it does seem one should be free to correct or clarify exception texts, certainly in a minor release, arguably even in a bug-fix release.

Hypothetically, someone could be using error messages to distinguish between errors they can handle and those they cannot. Although in that case we really ought to be using a custom error class to make it easier on those use cases (they should report that behavior to us anyways).

[mhvk]

Agreed that for that case we should have different error types (like we introduced UnitTypeError and UnitConversionError at some point -- though I think that was mostly for internal use).

One conclusion from that is that it is good to strongly encourage people to give feedback when things break!

[saimn]

In my opinion, how something is typeset (repr) or worded is not included in the API (for a minor release), so the wording of an error message is not, but the structure/type/behaviour of an object or the exception type is part of the API.

I agree with this definition. The main place where people may use the messages is tests, but breaking tests is not the same as breaking the code itself. There may be exceptions but it's hard for us to know, so indeed encouraging people to run weekly (?) tests with astropy-dev seems a good solution.

[taldcroft]

On error and warning messages specifically, in astropy we check those internally just to ensure we actually raise the right error, but really why would anyone rely on those outside of astropy?

The problem is that astropy very commonly uses generic exception types like ValueError and TypeError, so that the only way to catch a specific error is by using the exception message text.

We certainly could embark on a mission to improve that with custom error classes, but that is a different story.

[taldcroft]

Anyway, I really hope we can keep things more or less like they are for minor releases!

I agree 100% (as a culprit for some of the changes in question). I just think we need to ensure that such changes are appropriately communicated to stakeholders (putting on a manager's hat).

[taldcroft]

And I see now that I somewhat repeated https://github.com/astropy/astropy-APEs/pull/82#issuecomment-1441953700, so we're all in agreement.

[astrofrog]

In terms of communication, what if we required any API change in minor releases to be mentioned in the What's new? (For major releases we could just link to the API changes section in the changelog)

[saimn]

Yes I think we need to revisit the presentation of those changes in the changelog/whatsnew. E.g. for major releases Numpy has several sections: "Deprecations", "Expired deprecations", "Compatibility notes" (and "Performance improvements", cf another discussion on where to put those): https://numpy.org/doc/stable/release/1.24.0-notes.html Pandas has "Backwards incompatible API changes", "Deprecations" (and "Performance improvements"): https://pandas.pydata.org/docs/whatsnew/v1.5.0.html

[astrofrog]

Nice examples! I'll try and work something into this APE.

[astrofrog]

@pllim @saimn @taldcroft @mhvk - I've updated the APE to try and add some text that encapsulates some of the points above. Would you be able to take a look at it? Feel free to leave specific suggestions if you see text that should be added/modified/removed. Thanks!

[astrofrog]

@mhvk - I've updated the APE to now explicitly state that messages and reprs are not considered API. Let me know if you are happy with the text now!

@taldcroft - are you also happy with the wording?

@saimn @pllim - anything else you can think of that should be added to this? Are you happy with the text otherwise?

Once everyone here is happy I can mark this ready to review and send it out to astropy-dev :smile:

[taldcroft]

should the "What's new" for a major release be relative to that last major release?

In terms of features and bug fixes, I think that most people would expect it to be relative the last minor release. Since the major release no longer corresponds to an LTS there is no particular reason that many people would still be using it.

[taldcroft]

Now a big-picture comment that I don't see here yet. This is really superceding a big component of APE-2 which is the fundamental document on Astropy Release Cycle and Version Numbering. The second sentence in the abstract there mentions the LTS and will now be wrong.

To that end I propose integrating much of the policy-oriented content into APE-2 and following on with a substantial update to APE-2. That would be instead of an update to APE-2 which just points to APE-21. When people go to the Astropy APEs and open any of these documents they should reflect the current policy.

During the governance discussions we had a vision of policy documents, but I'm not sure that has really become reality. One test of that is that I don't actually know where I would find those policy documents. On the other hand, the APE's have continued as an effective and visible place for high level policy like release cycles, numbering, code formatting etc. So I think we should fully embrace the idea of APEs as living documents that get updated as needed to reflect the current policy.

An update to APE-2 to reflect this change would probably want to include a brief section on the prior policy so that people looking at APE-2 have an understanding of historical releases along with future ones.

[mhvk]

@taldcroft - OK, that makes sense, so the what's-new for a major release would mostly stand out in being one where one has to carefully read the API-change section.

I like the idea of living documents, as old versions are really easy to look at with git, but probably good to spell out the alternative: APE 2 would be formally archived and essentially become invisible (or marked with cross-out or so).

[astrofrog]

@taldcroft - so just to check, would APE 21 still exist to lay out the motivations etc for the change?

I'm not sure that relying on just 'git log' is really sufficient for people to browse old versions of APEs - instead I think we can point people to the versioned Zenodo entries?

[mhvk]

:+1: to linking to versioned Zenodo entries.

[taldcroft]

The first time I wrote the "big picture comment", I did think about just dropping APE-21. But then it seemed to me like it deserves its own APE as a substantial project change with a place to lay out the motivations and other usual APE things. Even the implementation / guideline details here can stay, but could also go into the APE-2 update largely verbatim.

For APE-2, I think the key thing is including some history in that document itself. This idea is a bit against our usual standard of doing everything with version control, but in this case for the typical reader of APE-2 it would be helpful to clearly see the history of the previous version. Basically that amounts to a few sentences along the lines of:

Prior to astropy version X.0.0, astropy supported the concept of Long Term Support releases which corresponded to a major release and were maintained with bug fixes for two years. For details see [link to the previous APE-2]. This policy was changed with the acceptance of [APE-21].

[eerovaher]

Should we consider the stacklevel of a warning to be part of the public API or not?

[pllim]

I got lost in these discussions. So, is the only blocker for this APE the debate on "what is public API"? Other details are ok?

[pllim]

Also @mwcraig said conda-forge maintenance would be easier with this APE because there is no longer a need to maintain LTS branch in astropy feedstock, so no more issues like https://github.com/conda-forge/astropy-feedstock/issues/118 .

And we can also probably get rid of "lts" build in ReadTheDocs.

[astrofrog]

@taldcroft - I started working on modifying APE 2 but the changes are going to be quite significant and it doesn't really feel like just an 'update' - there is text for instance giving the motivation etc for LTS which would need to go, and quite a bit of text would be changed, and e.g. the implementation section would no longer be relevant and so on.

While I think there are APEs that would work well with updates/additions/clarifications, APE 2 would be significantly changed beyond recognition. I wonder if it would make more sense to expand APE 21 so that it is a self-contained replacement of APE 2 and APE 2 could be updated to include 'Replaced by APE 21' near the top (PEPs have 'Status: Replaced' in some cases)

I'm happy to go with the majority opinion, but just wanted to bring up that if I have to update APE 2 it will be almost a complete re-write.

[astrofrog]

Having thought about this a bit more, it does make sense to update APE 2, so I've done that here.

I've also removed duplicated content here because I think it would be best to have things only live in one place (otherwise if APE 2 is updated in future, APE 21 may have guidelines inconsistent with APE 2).

[saimn]

About changelog/communication, I really like how Pandas did it for their recent major release: https://pandas.pydata.org/docs/whatsnew/v2.0.0.html All the useful information is there, and easy to navigate with clear sections etc.

[astrofrog]

@mhvk @taldcroft @nstarman - I lost track of whether we are in agreement with the current wording for exceptions/warnings. If not, please feel free to suggest specific changes to the current wording - thanks!

[eteq]

These are some consensus discussions from coordination meeting discussion on this APE - including me @nden @pllim @nstarman @weaverba137 @tepickering @tomdonaldson @WilliamJamieson @eerovaher

• need to clarify that the semantic step happens only if there are any pending->post-deprecaton
• (might already be clear: major step can happen in between if we break API)
• 6 months is too short for a deprecation period, 2 years is tool long.implies a minimum-deprecation of 1 years == 2 cycles
• Change to major/minor every two releases instead of four? Or more formally: every major release is yearly, minor releases are by default 6 mo after but can be more frequent if we want.
• This leads to actual breaking changes occuring every either one or one-and-a-half years
• (Might need to add something to "implementation" about the above)

Some of that might be a bit hard to understand - I'll follow up later (hopefully today) with some specific wording suggestions to implement this.

One other item to note that relates to the discussion: several observatory support staff in the room said something in the direction of "LTS was good before, but we are ok with this change after seeing the motivation and because the world has changed". So that's just more fodder of general support for this.

[astrofrog]

@mhvk @taldcroft - since you aren't mentioned in @eteq's previous comment and may not have been in on that discussion, are you on board with the suggestions in @eteq's comment? (just want to double check before going ahead and implementing the changes). Thanks!

[mhvk]

I agree with the deprecation period; I would not try to schedule major releases ahead of time (i.e., no need for a fixed schedule for those).

[taldcroft]

This looks good to me and I agree with @eteq's summary.

@mhvk - I'm not sure I understand "I would not try to schedule major releases ahead of time". I thought the plan is for a major release on a yearly cadence. This might be done with some thought to the timing of the yearly Python major release?

[mhvk]

@taldcroft - I was replying to

Change to major/minor every two releases instead of four? Or more formally: every major release is yearly, minor releases are by default 6 mo after but can be more frequent if we want.

I was just not sure the APE needs to decide on this; astropy seems mature enough it is not clear to me we need a major version bump every year. That said, going together with python updates might make a lot of sense. So, happy to go with whatever was discussed & decided at the meeting!

[taldcroft]

@mhvk - if we are strict about API policy and deprecations, I would guess that there will be a driver for a major update every year. But you are right that if by chance there are none in the changelog then there is no need for a major version bump.

[saimn]

I agree with the 1 year deprecation period, but not sure about the yearly major release : one of the goals of this APE was to keep the stability promise of the LTS for a ~2 years period. A major release is needed only to introduce breaking changes, i.e. removal of deprecations, or new feature that need an API break. Is there a need for a faster cadence ?

[pllim]

Is there a way to get some sort of consensus to move this forward?

[nstarman]

Is there a need for a faster cadence ?

As I understand it, we don't want deprecations to last for 2.5 years. If something is deprecated in v5.3 and can't be removed until v7 (because v6 is too early) that's too long if v6 to v7 is 2 years

[WilliamJamieson]

I agree with @nstarman, it will become difficult to maintain multiple versions of an API for ~2 years. I also agree any deprecation should exist for at least 1 year, so it is reasonable for us to allow for having a major release every year.

Since the goal is to drop LTS version support, we will then be expecting downstream users to migrate forward as we make minor releases. Consequently, downstream users should have at least 1 year of warning prior to a breaking change.