Skip to content

Conversation

@AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented Nov 13, 2025

This is derived from Hugo's #4704; see that PR for context.

This PR is an attempt to implement @sethmlarson's suggestion -- attempting to do this via review comments wasn't really going to work!

The ICS generation is now much simpler with no dependencies, simply outputting the required strings. I've also limited past events to 7 years to keep the file size smaller (for ~18months of pre-releases and a five-year lifespan). Realistically this could be much shorter, but I've been conservative.

A


📚 Documentation preview 📚: https://pep-previews--4705.org.readthedocs.build/

@AA-Turner AA-Turner requested a review from a team as a code owner November 13, 2025 20:03
@AA-Turner AA-Turner added the infra Core infrastructure for building and rendering PEPs label Nov 13, 2025
Comment on lines +72 to +74
global _PYTHON_RELEASES
if _PYTHON_RELEASES is not None:
return _PYTHON_RELEASES
Copy link
Member

Choose a reason for hiding this comment

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

I think chucking a @cache decorator on the function (from the original PR) is much cleaner than six lines to manage a global variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

@functools.cache is shorter but also overkill IMO, we don't need the LRU complexity etc.

Copy link
Member

Choose a reason for hiding this comment

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

It's not complex, it's a black box decorator :)

We don't need to worry what it's doing under the hood, the complexity that matters is the code we're looking at here.

And functools.cache is 1.4x as fast as the global.

Or if it's a problem, let's just ditch this caching altogether. It's only called three times, so it's not like it'll make any noticeable difference.

@AA-Turner AA-Turner requested a review from hugovk November 17, 2025 21:38
Comment on lines +72 to +74
global _PYTHON_RELEASES
if _PYTHON_RELEASES is not None:
return _PYTHON_RELEASES
Copy link
Member

Choose a reason for hiding this comment

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

It's not complex, it's a black box decorator :)

We don't need to worry what it's doing under the hood, the complexity that matters is the code we're looking at here.

And functools.cache is 1.4x as fast as the global.

Or if it's a problem, let's just ditch this caching altogether. It's only called three times, so it's not like it'll make any noticeable difference.

Copy link
Member

Choose a reason for hiding this comment

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

Let's also skip measuring coverage for if __name__ == '__main__': blocks, it's not worth testing.

I also suggested skipping https://github.com/python/peps/blob/main/release_management/__main__.py in the original PR. Is there anything special worth testing in there?

'X-WR-CALNAME:Python releases schedule',
]
for pep_number, release in releases:
normalised_stage = release.stage.casefold().replace(' ', '')
Copy link
Member

Choose a reason for hiding this comment

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

Why casefold?

These are all lowercase, trusted strings with predictable contents like "3.14.0 alpha 1", "3.14.0 candidate 3" and "3.14.5".

They won't have any uppercase or German "ß".

Suggested change
normalised_stage = release.stage.casefold().replace(' ', '')
normalised_stage = release.stage.replace(' ', '')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra Core infrastructure for building and rendering PEPs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants