Several PEPs: Normalise Post-History
#2375
Conversation
There was a problem hiding this comment.
The big issue I didn't think about before was if we use this instead of the initial proposed reST approach to solve #2358 , this processing isn't actually applied until if and when PEP 676 is accepted and fully implemented; until then, if we do format date-links as suggested here, they'll show up in a rather space-consuming and ugly style, rather than as inline links, which is a bit of a regression at least in aesthetics and space efficiency, if not functionality.
So, if we did want to go ahead with this rather than just using native reST links, we could use reST links for now, and then use a simple regex to convert them if and when PEP 676 goes live (since these links are just reST links without the backticks and trailing underscore, otherwise the format is identical.
Also, when I attempt to run this locally with a PEP containing a field formatted as specified, I get a TypeError on the union between the two types in the return type annotation.
$ python -b -X dev build.py -n
Running Sphinx v4.4.0
Traceback (most recent call last):
File "C:\Users\C. A. M. Gerlach\Documents\dev\Python\peps\build.py", line 75, in <module>
app = Sphinx(
File "C:\Miniconda3\envs\docrepr-env\lib\site-packages\sphinx\application.py", line 230, in __init__
self.setup_extension(extension)
File "C:\Miniconda3\envs\docrepr-env\lib\site-packages\sphinx\application.py", line 387, in setup_extension
self.registry.load_extension(self, extname)
File "C:\Miniconda3\envs\docrepr-env\lib\site-packages\sphinx\registry.py", line 433, in load_extension
mod = import_module(extname)
File "C:\Miniconda3\envs\docrepr-env\lib\importlib\__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 850, in exec_module
File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
File "C:\Users\C. A. M. Gerlach\Documents\dev\Python\peps\pep_sphinx_extensions\__init__.py", line 14, in <mod
from pep_sphinx_extensions.pep_processor.parsing import pep_parser
File "C:\Users\C. A. M. Gerlach\Documents\dev\Python\peps\pep_sphinx_extensions\pep_processor\parsing\pep_pars
, line 9, in <module>
from pep_sphinx_extensions.pep_processor.transforms import pep_headers
File "C:\Users\C. A. M. Gerlach\Documents\dev\Python\peps\pep_sphinx_extensions\pep_processor\transforms\pep_h
.py", line 122, in <module>
def _process_post_history(body: nodes.field_body) -> list[nodes.Text | nodes.reference]:
TypeError: unsupported operand type(s) for |: 'type' and 'type'conda list output:
$ conda list
# packages in environment at C:\Miniconda3\envs\docrepr-env:
#
# Name Version Build Channel
alabaster 0.7.12 py_0 conda-forge
astroid 2.9.0 py39hcbf5309_0 conda-forge
atomicwrites 1.4.0 pyh9f0ad1d_0 conda-forge
attrs 21.2.0 pyhd8ed1ab_0 conda-forge
babel 2.9.1 pyh44b312d_0 conda-forge
backcall 0.2.0 pyh9f0ad1d_0 conda-forge
backports 1.0 py_2 conda-forge
backports.functools_lru_cache 1.6.4 pyhd8ed1ab_0 conda-forge
beautifulsoup4 4.10.0 pyha770c72_0 conda-forge
bleach 4.1.0 pyhd8ed1ab_0 conda-forge
brotli 1.0.9 h8ffe710_6 conda-forge
brotli-bin 1.0.9 h8ffe710_6 conda-forge
brotlipy 0.7.0 py39hb82d6ee_1003 conda-forge
build 0.7.0 pyhd8ed1ab_0 conda-forge
ca-certificates 2021.10.8 h5b45459_0 conda-forge
certifi 2021.10.8 py39hcbf5309_1 conda-forge
cffi 1.15.0 py39h0878f49_0 conda-forge
cfgv 3.3.1 pyhd8ed1ab_0 conda-forge
charset-normalizer 2.0.9 pyhd8ed1ab_0 conda-forge
cmarkgfm 0.7.0 py39hb82d6ee_0 conda-forge
colorama 0.4.4 pyh9f0ad1d_0 conda-forge
cryptography 36.0.0 py39h7bc7c5c_0 conda-forge
cycler 0.11.0 pyhd8ed1ab_0 conda-forge
decorator 5.1.0 pyhd8ed1ab_0 conda-forge
distlib 0.3.4 pyhd8ed1ab_0 conda-forge
docrepr 0.2.0 dev_0 <develop>
docutils 0.17.1 py39haa95532_1
feedgen 0.9.0 pyh9f0ad1d_0 conda-forge
filelock 3.4.2 pyhd8ed1ab_1 conda-forge
fonttools 4.28.3 py39hb82d6ee_0 conda-forge
freetype 2.10.4 h546665d_1 conda-forge
furo 2021.10.9 pyhd8ed1ab_0 conda-forge
icu 68.2 h0e60522_0 conda-forge
identify 2.4.6 pyhd8ed1ab_0 conda-forge
idna 3.1 pyhd3deb0d_0 conda-forge
imagesize 1.3.0 pyhd8ed1ab_0 conda-forge
importlib-metadata 4.8.2 py39hcbf5309_0 conda-forge
importlib_metadata 4.8.2 hd8ed1ab_0 conda-forge
iniconfig 1.1.1 pyh9f0ad1d_0 conda-forge
intel-openmp 2021.4.0 h57928b3_3556 conda-forge
ipython 7.30.1 py39hcbf5309_0 conda-forge
isort 5.10.1 pyhd8ed1ab_0 conda-forge
jbig 2.1 h8d14728_2003 conda-forge
jedi 0.18.1 py39hcbf5309_0 conda-forge
jinja2 3.0.3 pyhd8ed1ab_0 conda-forge
jpeg 9d h8ffe710_0 conda-forge
keyring 23.4.0 py39hcbf5309_0 conda-forge
kiwisolver 1.3.2 py39h2e07f2f_1 conda-forge
lazy-object-proxy 1.7.1 py39hb82d6ee_0 conda-forge
lcms2 2.12 h2a16943_0 conda-forge
lerc 3.0 h0e60522_0 conda-forge
libblas 3.9.0 12_win64_mkl conda-forge
libbrotlicommon 1.0.9 h8ffe710_6 conda-forge
libbrotlidec 1.0.9 h8ffe710_6 conda-forge
libbrotlienc 1.0.9 h8ffe710_6 conda-forge
libcblas 3.9.0 12_win64_mkl conda-forge
libclang 11.1.0 default_h5c34c98_1 conda-forge
libdeflate 1.8 h8ffe710_0 conda-forge
libiconv 1.16 he774522_0 conda-forge
liblapack 3.9.0 12_win64_mkl conda-forge
libpng 1.6.37 h1d00b33_2 conda-forge
libtiff 4.3.0 hd413186_2 conda-forge
libxml2 2.9.12 hf5bbc77_1 conda-forge
libxslt 1.1.33 h65864e5_3 conda-forge
libzlib 1.2.11 h8ffe710_1013 conda-forge
loghub 0.5.1 pypi_0 pypi
lxml 4.7.1 py39h4fd7cdf_0 conda-forge
lz4-c 1.9.3 h8ffe710_1 conda-forge
markdown-it-py 2.0.1 pyhd8ed1ab_0 conda-forge
markupsafe 2.0.1 py39hb82d6ee_1 conda-forge
matplotlib 3.5.0 py39hcbf5309_0 conda-forge
matplotlib-base 3.5.0 py39h581301d_0 conda-forge
matplotlib-inline 0.1.3 pyhd8ed1ab_0 conda-forge
mccabe 0.6.1 py_1 conda-forge
mdit-py-plugins 0.3.0 pyhd8ed1ab_0 conda-forge
mdurl 0.1.0 pyhd8ed1ab_0 conda-forge
mkl 2021.4.0 h0e2418a_729 conda-forge
more-itertools 8.12.0 pyhd8ed1ab_0 conda-forge
munkres 1.1.4 pyh9f0ad1d_0 conda-forge
myst-parser 0.17.0 pyhd8ed1ab_0 conda-forge
nodeenv 1.6.0 pyhd8ed1ab_0 conda-forge
numpy 1.21.4 py39h6635163_0 conda-forge
olefile 0.46 pyh9f0ad1d_1 conda-forge
openjpeg 2.4.0 hb211442_1 conda-forge
openssl 1.1.1l h8ffe710_0 conda-forge
packaging 21.3 pyhd8ed1ab_0 conda-forge
parso 0.8.3 pyhd8ed1ab_0 conda-forge
pep517 0.12.0 py39hcbf5309_1 conda-forge
pickleshare 0.7.5 py_1003 conda-forge
pillow 8.4.0 py39h916092e_0 conda-forge
pip 22.0.2 pyhd8ed1ab_0 conda-forge
pkginfo 1.8.2 pyhd8ed1ab_0 conda-forge
platformdirs 2.3.0 pyhd8ed1ab_0 conda-forge
pluggy 1.0.0 py39hcbf5309_2 conda-forge
pre-commit 2.17.0 py39hcbf5309_0 conda-forge
prompt-toolkit 3.0.24 pyha770c72_0 conda-forge
py 1.11.0 pyh6c4a22f_0 conda-forge
pycparser 2.21 pyhd8ed1ab_0 conda-forge
pygments 2.11.2 pyhd8ed1ab_0 conda-forge
pylint 2.12.2 pyhd8ed1ab_0 conda-forge
pyopenssl 21.0.0 pyhd8ed1ab_0 conda-forge
pyparsing 3.0.6 pyhd8ed1ab_0 conda-forge
pyqt 5.12.3 py39hcbf5309_8 conda-forge
pyqt-impl 5.12.3 py39h415ef7b_8 conda-forge
pyqt5-sip 4.19.18 py39h415ef7b_8 conda-forge
pyqtchart 5.12 py39h415ef7b_8 conda-forge
pyqtwebengine 5.12.1 py39h415ef7b_8 conda-forge
pysocks 1.7.1 py39hcbf5309_4 conda-forge
pytest 6.2.5 py39hcbf5309_1 conda-forge
pytest-asyncio 0.17.2 pypi_0 pypi
python 3.9.7 h7840368_3_cpython conda-forge
python-dateutil 2.8.2 pyhd8ed1ab_0 conda-forge
python_abi 3.9 2_cp39 conda-forge
pytz 2021.3 pyhd8ed1ab_0 conda-forge
pywin32-ctypes 0.2.0 py39hcbf5309_1004 conda-forge
pyyaml 6.0 py39hb82d6ee_3 conda-forge
qt 5.12.9 h5909a2a_4 conda-forge
readme_renderer 27.0 pyh9f0ad1d_0 conda-forge
requests 2.26.0 pyhd8ed1ab_1 conda-forge
requests-toolbelt 0.9.1 py_0 conda-forge
rfc3986 1.5.0 pyhd8ed1ab_0 conda-forge
setuptools 59.4.0 py39hcbf5309_0 conda-forge
six 1.16.0 pyh6c4a22f_0 conda-forge
snowballstemmer 2.2.0 pyhd8ed1ab_0 conda-forge
soupsieve 2.3.1 pyhd8ed1ab_0 conda-forge
sphinx 4.4.0 pyh6c4a22f_1 conda-forge
sphinx-copybutton 0.5.0 pyhd8ed1ab_0 conda-forge
sphinx-inline-tabs 2022.1.2b11 pyhd8ed1ab_0 conda-forge
sphinxcontrib-applehelp 1.0.2 py_0 conda-forge
sphinxcontrib-devhelp 1.0.2 py_0 conda-forge
sphinxcontrib-htmlhelp 2.0.0 pyhd8ed1ab_0 conda-forge
sphinxcontrib-jsmath 1.0.1 py_0 conda-forge
sphinxcontrib-qthelp 1.0.3 py_0 conda-forge
sphinxcontrib-serializinghtml 1.1.5 pyhd8ed1ab_1 conda-forge
sqlite 3.37.0 h8ffe710_0 conda-forge
tbb 2021.4.0 h2d74725_1 conda-forge
tk 8.6.11 h8ffe710_1 conda-forge
toml 0.10.2 pyhd8ed1ab_0 conda-forge
tomli 2.0.0 pyhd8ed1ab_1 conda-forge
tornado 6.1 py39hb82d6ee_2 conda-forge
tqdm 4.62.3 pyhd8ed1ab_0 conda-forge
traitlets 5.1.1 pyhd8ed1ab_0 conda-forge
twine 3.7.1 pyhd8ed1ab_0 conda-forge
typing-extensions 4.0.1 hd8ed1ab_0 conda-forge
typing_extensions 4.0.1 pyha770c72_0 conda-forge
tzdata 2021e he74cb21_0 conda-forge
ucrt 10.0.20348.0 h57928b3_0 conda-forge
ukkonen 1.0.1 py39h2e07f2f_1 conda-forge
urllib3 1.26.7 pyhd8ed1ab_0 conda-forge
vc 14.2 hb210afc_5 conda-forge
virtualenv 20.13.0 py39hcbf5309_0 conda-forge
vs2015_runtime 14.29.30037 h902a5da_5 conda-forge
wcwidth 0.2.5 pyh9f0ad1d_2 conda-forge
webencodings 0.5.1 py_1 conda-forge
wheel 0.37.0 pyhd8ed1ab_1 conda-forge
win_inet_pton 1.1.0 py39hcbf5309_3 conda-forge
wrapt 1.13.3 py39hb82d6ee_1 conda-forge
xz 5.2.5 h62dcd97_1 conda-forge
yaml 0.2.5 h8ffe710_2 conda-forge
zipp 3.6.0 pyhd8ed1ab_0 conda-forge
zlib 1.2.11 h8ffe710_1013 conda-forge
zstd 1.5.0 h6255e5f_0 conda-forge
| node = nodes.reference("", | ||
| date.strip(), | ||
| refuri=uri.strip(" \f\n\r\t><"), | ||
| internal=False |
There was a problem hiding this comment.
| internal=False | |
| internal=False, |
Minor nit
True, although we could backport it as we have a few other changes, it would be fairly easy. A |
I mean, we currently declare 3.9 compat. I'm not sure we should require bleeding edge Python, especially when the cost of a future import is fairly trivial.
Yeah, though it would be effort that will have little long-term value. |
|
Also, what does "PRS" stand for? I've never heard that abbreviation before... |
"PEP Rendering System"
Agree. Hopefully 676 will have a decision soon.
Yes, I'll add one. 3.10's "bleeding edge" status can be argued later, I'm not requiring 3.11 quite yet ;-) A |
|
PEP Rendering System? Programming Reusable Sphinxes? Python-Related Services? |
| refuri=uri.strip(" \f\n\r\t><"), | ||
| internal=False | ||
| ) | ||
| except ValueError: |
There was a problem hiding this comment.
Should we instead fail if the field doesn't parse properly? It would be good to give that feedback to authors.
There was a problem hiding this comment.
Unless we update every single current PEP to the inline link format, there would be quite a lot of failures!
A
There was a problem hiding this comment.
True, I guess we need some way to avoid erroring on anything that's already there, while alerting on things that look like the desired syntax but aren't quite the same.
Also, wouldn't this get a false positive on PEP 3135:
pep-3135.txt:Post-History: 28-Apr-2007, 29-Apr-2007 (1), 29-Apr-2007 (2), 14-May-2007, 12-Mar-2009
There was a problem hiding this comment.
Also PEP 551:
pep-0551.rst:Post-History: 24-Aug-2017 (security-sig), 28-Aug-2017 (python-dev)
There was a problem hiding this comment.
Perhaps a normalisation pass is needed here. (Or ignoring anything that doesn't start with "http")
A
There was a problem hiding this comment.
I wouldn't mind us changing the handful of PEPs with very eccentric Post-History formats (648, 538, 467, 367, 311 also stand out; and 424 and 428 have bare URLs).
There was a problem hiding this comment.
I certainly don't mind updating the outlier PEPs that don't come close to conforming to the format that has always been specified in PEP 1, and could potentially lead to parsing issues with the new processing system. This would allow stricter validation for (presumably new/updated) PEPs that appear to be at least attempting to conform to the new style. E.g. if Post-History contains either [<>] or https?:, its entries must, at minimum. parse without error.
I can then add a pygrep linter check that checks such Post-Historyies that do qualify for whether they match the expected format, to detect common mistakes that would be technically syntactically valid but semantically not produce the intended result.
There was a problem hiding this comment.
I'm not sure either of us were suggesting you do as much work as you did, but I don't disagree with conforming them since it makes it simpler for us in processing, linting and providing useful immediate feedback to user (and not failing silently), regardless of which format we go with. And for now, only a few would need to be converted either way
But in any case, now that this is done, if we do go ahead with this approach, I'd suggest putting only the single actual intended line (splitting on " ") in the try-block, so we don't silently ignoring other errors here, and the rest in the else block.
If we're going to use prefixes for infra PRs, can we not use cryptic, bespoke, made-up initialisms instead of widely-used plain language abbreviations like "Infra" here? Particularly ones that are easy to confuse with "PRs" (which is how I read it). Acronyms Seriously Suck (and I work for NASA, I should know). |
|
I broke up the Post-History changes into four commits with rough categories, and also implemented the change in A |
There was a problem hiding this comment.
At least per my testing, the custom processing is still not triggering for the legacy script, but is for the same PEP for the Sphinx system, after pulling this PR branch and re-running both—the links are still rendered in plain text, just as before.
Per the discussion on #2358, to move forward I propose we:
- Merge PEP 1 & 12: Have Post-History link posts to preserve Discussions-To history #2358 specifying a simplified, structured subset of the reST syntax for links
- Drop the bespoke processing part of this PR
- Retain the normalizations in this PR, converting the few that were linkified to the reST-based format, and merge that
- I follow up with a PR (already prototyped) adding automated linting checks for it, like for the other headers
Alternatively, if we do decide to go with this approach instead, we should be more strict about how we handle errors so they don't pass silently, and I'll need to do more testing/checking of various scenarios.
| refuri=uri.strip(" \f\n\r\t><"), | ||
| internal=False | ||
| ) | ||
| except ValueError: |
There was a problem hiding this comment.
I'm not sure either of us were suggesting you do as much work as you did, but I don't disagree with conforming them since it makes it simpler for us in processing, linting and providing useful immediate feedback to user (and not failing silently), regardless of which format we go with. And for now, only a few would need to be converted either way
But in any case, now that this is done, if we do go ahead with this approach, I'd suggest putting only the single actual intended line (splitting on " ") in the try-block, so we don't silently ignoring other errors here, and the rest in the else block.
|
Hey @AA-Turner , given our discussion on #2358 , looks like this needs dropping the first three commits for now, and updating the links in the "extra information" and "bare hyperlinks" commits to use the slightly different format. Would you prefer to take care of that, or would you like me to here? I could also open another PR, but I'd rather avoid pinging everybody again. Once this is merged, I have a PR tested and ready to go revising the existing header validation checks to ensure both this header and the others are correct and programmatically parsable, as well as clarifying a few inconsistencies/ambiguities in this regard in PEP 1/12/the templates and updating a handful of remaining PEPs that don't follow the rules. This will allow both us and others to do any number of useful things programmatically in the future, both here and externally, without further messing with the existing PEPs or requiring workarounds or hardcoding. |
AA-Turner
force-pushed
the
post-history-links
branch
from
0c19794 to
e45cce9
Compare
AA-Turner
changed the title
Post-History
|
This is waiting for #2358 to be resolved (as it now uses the reST link format proposed by that PR). A |
pep-0428.txt
Outdated
| @@ -8,7 +8,7 @@ Type: Standards Track | |||
| Content-Type: text/x-rst | |||
| Created: 30-Jul-2012 | |||
| Python-Version: 3.4 | |||
| Post-History: https://mail.python.org/pipermail/python-ideas/2012-October/016338.html | |||
| Post-History: `05-Oct-2012 <https://mail.python.org/pipermail/python-ideas/2012-October/016338.html>`__` | |||
There was a problem hiding this comment.
| Post-History: `05-Oct-2012 <https://mail.python.org/pipermail/python-ideas/2012-October/016338.html>`__` | |
| Post-History: `05-Oct-2012 <https://mail.python.org/pipermail/python-ideas/2012-October/016338.html>`__ |
AA-Turner
removed request for
vstinner,
warsaw,
benjaminp,
encukou,
scoder,
njsmith,
ronaldoussoren,
ncoghlan,
iritkatriel,
pfmoore,
eliben,
ericsnowcurrently,
pganssle,
theller,
rhettinger,
zooba,
pitrou,
gvanrossum,
Mariatta,
ethanfurman,
markshannon,
pablogsal,
ilevkivskyi,
larryhastings,
tim-one,
terryjreedy,
brandtbucher and
isidentical
For Post-History fields of the form proposed in #2358 (comment)
A