Several PEPs: Normalise Post-History#2375
Uh oh!
There was an error while loading. Please reload this page.
Conversation
CAM-Gerlach left a comment
CAM-Gerlach left a comment 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.
Details
$ 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_extensionself.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_modulereturn _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 <modfrom 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:
Details
$ 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
AA-Turner commented Mar 2, 2022
True, although we could backport it as we have a few other changes, it would be fairly easy. A |
CAM-Gerlach commented Mar 2, 2022
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. |
CAM-Gerlach commented Mar 2, 2022
Also, what does "PRS" stand for? I've never heard that abbreviation before... |
AA-Turner commented Mar 2, 2022
"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 |
JelleZijlstra commented Mar 2, 2022
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.
CAM-Gerlach commented Mar 2, 2022
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). |
AA-Turner commented Mar 2, 2022
I broke up the Post-History changes into four commits with rough categories, and also implemented the change in A |
CAM-Gerlach left a comment
CAM-Gerlach left a comment 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 #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.
CAM-Gerlach commented Mar 8, 2022
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-Turnerforce-pushed the post-history-links branch from 0c19794 to e45cce9CompareAA-Turner changed the title Post-HistoryAA-Turner commented Mar 8, 2022
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
| 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 Mariatta, benjaminp, brandtbucher, eliben, encukou, ericsnowcurrently, ethanfurman, gvanrossum, ilevkivskyi, iritkatriel, isidentical, larryhastings, markshannon, ncoghlan, njsmith, pablogsal, pfmoore, pganssle, pitrou, rhettinger, ronaldoussoren, scoder, terryjreedy, theller, tim-one, vstinner, warsaw and zooba
4 participants
For Post-History fields of the form proposed in #2358 (comment)
A