Skip to content

Infra: Use a list of authors in peps.json - #4226

Merged
AA-Turner merged 7 commits into
python:mainfrom
AA-Turner:comma
Feb 3, 2025
Merged

Infra: Use a list of authors in peps.json#4226
AA-Turner merged 7 commits into
python:mainfrom
AA-Turner:comma

Conversation

@AA-Turner

@AA-TurnerAA-Turner commented Jan 24, 2025

Copy link
Copy Markdown
Member

Closes#4211.

Alternatively, we could just use a different seperator (;)? The authors field of peps.json was originally intended for literal reproduction without any re-parsing, hence why I joined the alreay-parsed list of authors into a string.

A


📚 Documentation preview 📚: https://pep-previews--4226.org.readthedocs.build/peps.json

@AA-TurnerAA-Turner added the infra Core infrastructure for building and rendering PEPs label Jan 24, 2025
@AA-Turner
AA-Turner requested a review from a team as a code ownerJanuary 24, 2025 00:27
@AA-Turner
AA-Turner requested review from hugovk and removed request for a teamJanuary 24, 2025 00:27
@hugovk

Copy link
Copy Markdown
Member

I think the first question is if we want to break backwards compatibility?

I lean to not breaking compatibility. If so, the other options are:

  • keep the authors string using a different separator (for example, ";")
  • keep the authors string and add a new array of author strings

@warsaw

Copy link
Copy Markdown
Member

keep the authors string and add a new array of author strings

I'd vote for keeping authors as is, and adding (something like) author_list as the array of author strings to keep backward compatibility.

@AA-Turner

Copy link
Copy Markdown
MemberAuthor

From a cursory search, it seems the vast majority of instances of peps.json in the wild are reproductions of the 723 example:

https://github.com/search?q=%22https%3A%2F%2Fpeps.python.org%2Fapi%2Fpeps.json%22+NOT+is%3Afork+NOT+is%3Aarchived&type=code

But sure, happy to add as a new field. The only negative would be the file size (~15% increase).

A

@hugovkhugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From a cursory search, it seems the vast majority of instances of peps.json in the wild are reproductions of the 723 example:

github.com/search?q=%22https%3A%2F%2Fpeps.python.org%2Fapi%2Fpeps.json%22+NOT+is%3Afork+NOT+is%3Aarchived&type=code

Here's a couple that would have broken (but I'm sure we could have convinced the authors to update :)

But sure, happy to add as a new field. The only negative would be the file size (~15% increase).

Yep, I think 331K -> 368K is okay. Thanks!

Comment threadpep_sphinx_extensions/pep_zero_generator/parser.py
Comment threadpep_sphinx_extensions/pep_zero_generator/parser.py
Comment threadpeps/api/index.rst Outdated
Comment threadpeps/api/index.rst
@hugovkhugovk changed the title Use a list of authors in peps.jsonInfra: Use a list of authors in peps.jsonJan 25, 2025

@hugovkhugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

Comment threadpeps/api/index.rst
The structure is like:

.. code-block:: javascript
.. code-block:: typescript

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does typescript give nicer formatting than javascript?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, especially since we now use Array<...>

Current:

image

Proposed:

image

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

Labels

infraCore infrastructure for building and rendering PEPs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PEPs API: handle name with commas in CSV author list

3 participants

@AA-Turner@hugovk@warsaw