Add support for topic indices#2579

AA-Turner · 2022-05-05T23:59:10Z

Draft, as a proof of concept to show the rendered preview.

I'm going to seperate the PEP changes into a distinct PR from the rendering infastructure changes, but I included them here so as to trigger the sub-index generation.

I haven't updated the tests or anything so they'll likely fail.

Rendered packaging sub-index preview:

https://pep-previews--2579.org.readthedocs.build/topic/packaging/

A

ghost · 2022-05-05T23:59:11Z

The following commit authors need to sign the Contributor License Agreement:

9087854+aa-turner@users.noreply.github.com

Click the button to sign:

AA-Turner · 2022-05-06T00:02:04Z

Odd the CLA bot pinged up again -- were CLA signing statuses not migrated from the 'old' model? That's the only thing I can think of that might be causing it to not like me.

A

AA-Turner · 2022-05-06T00:20:16Z

@pradyunsg for feedback on the packaging side -- I'd appreciate views. Would a simpler page just with a numerical index suffice? Are the categorisations useful?

A

JelleZijlstra

Thanks for working on this!

JelleZijlstra · 2022-05-06T00:23:06Z

pep_sphinx_extensions/pep_zero_generator/subindicies.py

@@ -0,0 +1,50 @@
+"""Utilities to support sub-indices for PEPs."""


The filename is misspelled (subindicies -> subindices)

Perils of typing past midnight! Thanks for noticing.
A

Also in function/variable names in this file.
And it would be clearer to use the same naming as the display URL, so topics instead of subindices

And it would be clearer to use the same naming as the display URL, so topics instead of subindices
The one issue with that, though, is that it can quite easily be extended to generate subindicies filtered on other headers than Topic, e.g. Type, Status, a future such field or potentially something else; the code isn't topic-specific.

JelleZijlstra · 2022-05-06T00:24:28Z

The preview is live at https://pep-previews--2579.org.readthedocs.build/topic/packaging/. It's missing links.

AA-Turner · 2022-05-06T00:28:03Z

It's missing links.

Yep, I just realised the PEP 0 transforms add the links later. In principle there's no reason we can't add said links during the PEP 0 creation process, I'll make that change when I go through the next round of updates.

A

CAM-Gerlach · 2022-05-06T02:07:40Z

Yep, I just realised the PEP 0 transforms add the links later. In principle there's no reason we can't add said links during the PEP 0 creation process, I'll make that change when I go through the next round of updates.

The same is true of the email address obfuscation, BTW, and the Page Source link.

CAM-Gerlach · 2022-05-06T03:04:17Z

I figure you're also aware of the issue that the index by category is not being generated at all on the main PEP 0 (while it is on the Packaging sub-index)?

Also, its currently impossible to find the topic-specific indices as nothing links to them. I assume you're already aware of this, but to address it, I suggest a "Index by topic" heading immediately below the introduction with a generated list of links to each topic, something like the following:

Indices by Topic

View sub-indices containing just PEPs belonging to specialized topic areas.

CAM-Gerlach

Thanks for doing this. A handful of comments on the code as it stands now; some minor and some more substantive.

I can add it to the linters and PEP 1/12/template in a separate PR, if desired.

pep_sphinx_extensions/pep_zero_generator/parser.py

CAM-Gerlach · 2022-05-06T02:50:32Z

pep_sphinx_extensions/pep_zero_generator/pep_index_generator.py

+if (len(file_path.stem) ==8
+andfile_path.stem.startswith("pep-")
+andfile_path.suffixin{".txt", ".rst"}):


How come the matching logic was downgraded to be less precise and correct than before, yet longer/more complex? Previously, only actual PEPs following the prescribed pattern (pep-\d{4}\.(txt|rst)) were matched, while this matches any 8-character txt or rst file that starts with pep-.

pep_sphinx_extensions/pep_zero_generator/parser.py

CAM-Gerlach · 2022-05-06T02:54:16Z

pep_sphinx_extensions/pep_zero_generator/pep_index_generator.py

-out_dir=Path(app.outdir) /"api"
-out_dir.mkdir(exist_ok=True)
-Path(out_dir, "peps.json").write_text(pep0_json, encoding="utf-8")
+Path(app.outdir, "peps.json").write_text(create_pep_json(peps), encoding="utf-8")


The previous JSON API path logic was inexplicably removed here, and the peps.json is now no longer generated at the correct path.

pep_sphinx_extensions/pep_zero_generator/subindicies.py

pep_sphinx_extensions/pep_zero_generator/pep_index_generator.py

pep_sphinx_extensions/pep_zero_generator/subindicies.py

pep_sphinx_extensions/pep_zero_generator/writer.py

hugovk · 2022-05-06T06:11:53Z

Odd the CLA bot pinged up again -- were CLA signing statuses not migrated from the 'old' model? That's the only thing I can think of that might be causing it to not like me.
A

Yes but now it's checking by email address.

If you committed with a ID+username@users.noreply.github.com email, that also needs to be on the CLA list.

Can you do the CLA thing with that address too?

hugovk

https://pep-previews--2579.org.readthedocs.build/topic/packaging/ is the new index.

https://pep-previews--2579.org.readthedocs.build/topic/ is a 404, shall we generate a simple list of topics?

hugovk · 2022-05-06T08:46:48Z

pep_sphinx_extensions/pep_zero_generator/subindicies.py

@@ -0,0 +1,50 @@
+"""Utilities to support sub-indices for PEPs."""


Also in function/variable names in this file.
And it would be clearer to use the same naming as the display URL, so topics instead of subindices

pradyunsg · 2022-05-06T12:34:40Z

Thanks for picking this up and turning this around so quickly @AA-Turner! ^>^

Would a simpler page just with a numerical index suffice? Are the categorisations useful?

I think a numerical index would be significantly less useful than the (as-currently-implemented) categorised index. :)

The categorisation makes it much more obvious what the state of every PEP is, what's "in-flight" and what each status means has something that has been slightly frustrating to communicate -- this communicates that very clearly.

Just looking at this index has flagged a few minor things, that I'll follow up on separately (like a few of the "old" Informational PEPs should really be Standards Track and Final + PEP 609's title is 50% acronym which isn't great).

Beyond that, I have a few more bits of feedback/additional requests. We can defer these to separate follow ups, if any of these are along the lines of "we should discuss this in a broader context" or "this is complicated to implement". :)

I think the Packaging page would benefit significantly from having a more detailed/specific introduction, noting the following details about Packaging PEPs:
- The actual specifications are maintained at https://packaging.python.org/en/latest/specifications/ -- these PEPs are supposed to only be for to introduce new ones or make substantial changes to existing specifications.
- That the "Specification Update Process" is documented in the PyPA's documentation site.
Doing this in a scalable and maintainable manner was what motivated my suggestion to use a Sphinx directive for generating the index entries. :)
Could Open PEPs be listed before Accepted PEPs? That way, a new PEP would basically only flow "down" the page which would be nice. :)
Having a page on the subfolder root (i.e. topic/index.html) would be useful. It would only list packaging for now -- but that's better than a 404 IMO. To be a bit of a broken record: Instead of generating pages in-memory, having these pages be regular .rst files that we put directives into would make this nicer + more obvious.

/cc @pfmoore and @dstufft as the standing PEP delegates, in case they have inputs.

CAM-Gerlach · 2022-05-06T19:38:26Z

https://pep-previews--2579.org.readthedocs.build/topic/ is a 404, shall we generate a simple list of topics?

Yeah, we could add a toctree directive in the generated source file (or do it in a transform, etc).

Would a simpler page just with a numerical index suffice? Are the categorisations useful?
I think a numerical index would be significantly less useful than the (as-currently-implemented) categorised index. :)

Somehow, I missed this, but I agree with @pradyunsg —currently, due to what I presume is a logic bug (with suggested fix above), this PR actually does the opposite, it no longer generates the categories at all for the main PEP 0 index, only the subindicies.

I think the Packaging page would benefit significantly from having a more detailed/specific introduction, noting the following details about Packaging PEPs:

This might be helpful, but it requires some special-casing for the Packaging Topic, when at least the present approach seems to be treating Topic more as a sort filter rather than a special case (as more so did the original Track proposal). If that's the case, making the list of Topics into a lookup table dict mapping to custom descriptions would probably be the most extensible approach.

Could Open PEPs be listed before Accepted PEPs? That way, a new PEP would basically only flow "down" the page which would be nice. :)

That makes sense; currently, the ordering is perhaps a bit haphazard.

To be a bit of a broken record: Instead of generating pages in-memory, having these pages be regular .rst files that we put directives into would make this nicer + more obvious.

Yes, but how does this scale with additional topics? If it was only for PEP 0, or perhaps even a single topic page I'd agree, but single-sourcing the topic names (and descriptions, if necessary) in one data structure and then generating them all via one consistent code path seems a lot simpler, cleaner and DRYer than manually hardcoding a bunch of duplicative rst files with the same structure and boilerplate for the index and each subindex, not to mention harder to maintain.

Adding a new topic would require creating a whole new file with the right structure, content and directives, plus manually adding it to the index, rather than just adding a name and description in a common data structure; updating something in the format/structure requires changing the same thing many different places, versus just a few lines of code. And meanwhile, we'd still have to draw the line somewhere in terms of generated versus baked-in content.

pradyunsg · 2022-05-06T21:04:15Z

If that's the case, making the list of Topics into a lookup table dict mapping to custom descriptions would probably be the most extensible approach.

That's the least disruptive way to implement this, I think.

I'm gonna leave out my response to @CAM-Gerlach's comments on the directives suggestion I've made -- I'd prefer to not flood this PR with discussion in a not-critical-path refactoring suggestion. :)

AA-Turner · 2022-05-07T11:41:34Z

The CLA website is also reporting an internal server error with the link on this PR...

A

AA-Turner · 2022-05-07T11:52:51Z

https://pep-previews--2579.org.readthedocs.build/topic/packaging/

@pradyunsg how's the additional description?

On your other points:

I'm not going to change the category order in this PR.
I agree with Cam about not using a custom directive, for the reasons he mentioned and as I'm averse to "non-native" reST, as it makes things less discoverable.
I added a toctree directive for the topic page.

I didn't have time today to update the PEP 0 transforms, though.

A

pep_sphinx_extensions/pep_zero_generator/subindices.py

pep_sphinx_extensions/pep_zero_generator/constants.py

CAM-Gerlach · 2022-05-07T23:18:49Z

I agree with Cam about not using a custom directive, for the reasons he mentioned and as I'm averse to "non-native" reST, as it makes things less discoverable.

Actually, for the record, the solution he proposed was much more of a "native" reST approach and easier to discover, as it uses a reST directive and normal rst source files rather than generating the source files in arbitrary Python code and manually adding them to the files to be built, burying the source text within the code and spread between several different places. After looking over a more detailed version of his proposal, I think its actually pretty workable with only a small amount of static duplication, but can be left for a separate future PR.

AA-Turner · 2022-05-07T23:24:35Z

"native" reST approach

By this I mean directives as specified in the reStructuredText specification -- having custom unrecognised directives isn't very useful to e.g. the GitHub UI, which uses Docutils (or a reimplementation of it).

I'll stop before I drift further off topic, though.

A

CAM-Gerlach · 2022-05-08T00:14:57Z

By this I mean directives as specified in the reStructuredText specification

The stated purpose of directives are to be an extension mechanism for reST without having to go outside of it by using manual hacks like this. Quoting the first line of the official reST specification defining directives:

Directives are an extension mechanism for reStructuredText, a way of adding support for new constructs without adding new primary syntax (directives may support additional syntax locally).

Yes, they aren't a first-party directive understood by the reference parser (docutils), but neither are Sphinx extensions, nor our custom :pep:, :rfc:, etc. roles, and certainly not arbitrary Python code, as it is currently implemented.

having custom unrecognised directives isn't very useful to e.g. the GitHub UI, which uses Docutils (or a reimplementation of it).

I'm a little confused how arbitrary custom Python code that generates files from scratch that don't even exist at all in the source is more compatible with GitHub and other consumers (never mind the same is true for any other Sphinx extension, not to mention all the other custom roles, transforms, output translations, theme HTML/CSS/JS, and everything else we have)? 🤣

AA-Turner · 2022-06-08T19:27:19Z

Is there a link from https://pep-previews--2579.org.readthedocs.build/ to the topics?

https://pep-previews--2579.org.readthedocs.build/topic?

A

hugovk

Is there a link from pep-previews--2579.org.readthedocs.build to the topics?
pep-previews--2579.org.readthedocs.build/topic?

Do we need one from https://pep-previews--2579.org.readthedocs.build/ to https://pep-previews--2579.org.readthedocs.build/topic/ ? I don't remember if this came up already :)

Not particularly important, can always add it later if needed.

Thank you all for this, approved!

Would be good for @pradyunsg to have a quick check too.

ambv · 2022-06-09T14:26:09Z

Hi there, I temporarily added DO-NOT-MERGE as I need this open to squash the CLA email problem.

AA-Turner · 2022-06-11T21:54:36Z

python/cpython#93736 passed the CLA check...

pradyunsg

Looks good to me!

Thanks for waiting on my inputs and addressing my earlier feedback! :)

JelleZijlstra · 2022-06-14T21:21:34Z

@ambv could you use a different PR by @AA-Turner to figure out the CLA issue? I'd like to get this PR in because it'll help the packaging people and it's been waiting for a long time already.

AA-Turner · 2022-06-14T21:25:03Z

I made a new PR with an empty commit at #2650 that fails CLA checks -- @ambv does that one work for your testing?

A

pradyunsg · 2022-06-15T19:14:55Z

This needs a closes #2572 in the PR description. :)

pradyunsg · 2022-06-18T16:36:44Z

@AA-Turner could you merge or rebase this, such that the changes in #2636 is included in the PR preview on this PR?

pradyunsg · 2022-06-18T19:16:03Z

Whee! Thanks everyone for helping get this implemented, polished and over the line! ✨

https://peps.python.org/topic/packaging/

* main: (47 commits) PEP 668: Address feedback and mark as accepted (python#2673) PEP-0691 Gramatical changes + `meta` key description under Project List (python#2677) PEP691: Mark Accepted + Grammar Fixes + Small Fix (python#2674) PEP 691: touch up (python#2668) PEP 650: Withdraw and move to Standards Track (python#2665) PEP 561: Mark as final (python#2663) PEP 660: Mark as Final (python#2664) PEP 615: Fix incorrect RFC link (python#2662) Multiple PEPs: Move Packaging PEPs to Standards Track and mark as Final (python#2657) PEP 671: Since it keeps getting asked about, add a para on deferreds (python#2661) Infra: Tweak PEP references to work on topic sub-index pages (python#2658) PEP 632: Remove `Topic: Packaging` header (python#2656) Infra: Make colour theme cycler button accessible (python#2619) PEP 593: Fix citation references (python#2640) PEP 553: Fix citation references (python#2639) PEP 668: Add PEP-Delegate (python#2654) PEP 693: Python 3.12 Release Schedule (python#2648) Add support for topic indices (python#2579) PEP691: Switch to a List for Project, Address more Feedback (python#2653) PEP 671: Add section on evaluation order (python#2652) ...

AA-Turner added the DO-NOT-MERGE label May 5, 2022

AA-Turner force-pushed the subindicies branch from af2e840 to cb74fbfCompare May 6, 2022 00:11

AA-Turner self-assigned this May 6, 2022

JelleZijlstra reviewed May 6, 2022
View reviewed changes

CAM-Gerlach linked an issue May 6, 2022 that may be closed by this pull request
Sub-index pages for specific categories of PEPs (Packaging, etc) #2572
Closed

CAM-Gerlach reviewed May 6, 2022
View reviewed changes

CAM-Gerlach added the infra Core infrastructure for building and rendering PEPs label May 6, 2022

hugovk reviewed May 6, 2022
View reviewed changes

hugovk changed the title ~~Add support for sub-indicies~~Infra: Add support for sub-indiciesMay 6, 2022

pradyunsg mentioned this pull request May 6, 2022
Sub-index pages for specific categories of PEPs (Packaging, etc) #2572
Closed

AA-Turner mentioned this pull request May 7, 2022
Refactor peps.json logic into PEP class #2585
Merged

CAM-Gerlach reviewed May 7, 2022
View reviewed changes

pep_sphinx_extensions/pep_zero_generator/subindices.py Outdated Show resolvedHide resolved

CAM-Gerlach reviewed May 7, 2022
View reviewed changes

pep_sphinx_extensions/pep_zero_generator/constants.py Outdated Show resolvedHide resolved

CAM-Gerlach mentioned this pull request May 8, 2022
Decouple and unify PEP header processing for rendering, PEP 0, JSON, RSS and linting #2587
Open

AA-Turner added 2 commits June 8, 2022 20:28

One more
991ba56

capitalisation
a32af64

hugovk approved these changes Jun 8, 2022
View reviewed changes

Add note about the topic index
57d3ca3

AA-Turner changed the title ~~Infra: Add support for sub-indicies~~Add support for topic indicesJun 8, 2022

AA-Turner closed this Jun 9, 2022

AA-Turner reopened this Jun 9, 2022

ambv added the DO-NOT-MERGE label Jun 9, 2022

AA-Turner closed this Jun 11, 2022

AA-Turner reopened this Jun 11, 2022

pradyunsg approved these changes Jun 14, 2022
View reviewed changes

AA-Turner mentioned this pull request Jun 16, 2022
Update Sphinx config python/devguide#898
Merged

Merge branch 'main' into subindicies
9289f9d

ambv enabled auto-merge (squash) June 18, 2022 17:33

ambv removed the DO-NOT-MERGE label Jun 18, 2022

ambv disabled auto-merge June 18, 2022 18:04

ambv merged commit a2f2d6c into python:mainJun 18, 2022

pradyunsg mentioned this pull request Jun 19, 2022
Running build.py to render PEPs leaves residual files in a new topic/ folder #2659
Closed

erlend-aasland mentioned this pull request Jun 27, 2022
pep 687/mark as accepted erlend-aasland/peps#2
Closed

CAM-Gerlach mentioned this pull request Jun 29, 2022
Link topic page(s) in the rendered topic header #2688
Closed

hugovk mentioned this pull request Jan 30, 2023
PEP 404: Add Python 2.8 to Release PEPs topic #2986
Merged

		@@ -0,0 +1,50 @@
		"""Utilities to support sub-indices for PEPs."""

Uh oh!

Add support for topic indices#2579

Add support for topic indices #2579

Uh oh!

Conversation

AA-Turner commented May 5, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rendered packaging sub-index preview:

Uh oh!

ghost commented May 5, 2022• edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AA-Turner commented May 6, 2022

Uh oh!

AA-Turner commented May 6, 2022

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

JelleZijlstraMay 6, 2022

Choose a reason for hiding this comment

Uh oh!

AA-TurnerMay 6, 2022

Choose a reason for hiding this comment

Uh oh!

hugovkMay 6, 2022

Choose a reason for hiding this comment

Uh oh!

CAM-GerlachMay 6, 2022

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra commented May 6, 2022• edited by AA-Turner Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AA-Turner commented May 6, 2022

Uh oh!

CAM-Gerlach commented May 6, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CAM-Gerlach commented May 6, 2022

Indices by Topic

Uh oh!

CAM-Gerlach left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CAM-GerlachMay 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CAM-GerlachMay 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugovk commented May 6, 2022

Uh oh!

hugovk left a comment

Choose a reason for hiding this comment

Uh oh!

hugovkMay 6, 2022

Choose a reason for hiding this comment

Uh oh!

pradyunsg commented May 6, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CAM-Gerlach commented May 6, 2022

Uh oh!

pradyunsg commented May 6, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AA-Turner commented May 7, 2022

Uh oh!

AA-Turner commented May 7, 2022

Uh oh!

Uh oh!

Uh oh!

AA-Turner commented May 5, 2022•
edited
Loading

ghost commented May 5, 2022•
edited by ghost
Loading

JelleZijlstra commented May 6, 2022•
edited by AA-Turner
Loading

CAM-Gerlach commented May 6, 2022•
edited
Loading

pradyunsg commented May 6, 2022•
edited
Loading

pradyunsg commented May 6, 2022•
edited
Loading

CAM-Gerlach commented May 8, 2022•
edited
Loading