blurb: replace spaces with underscores in news directory

[hugovk]

Fixes #186.

The spaces in blurb's "C API" and "Core and Builtins" directories are problematic, and replacing them would:

• simplify tooling, for example gh-101100: Test only Doc/ files in nit-picky mode cpython#103019 (comment)
• allow selecting paths in terminal with mouse

#186 suggested replacing with underscores. I suggest replacing with dashes:

• consistent with existing "Tools/Demos" -> "Tools-Demos"
• underscores are sometimes considered part of a word: https://blog.codinghorror.com/of-spaces-underscores-and-dashes/

Blurb has two functions for converting in each direction:

• sanitize_section()
• unsanitize_section()

This PR first adds some tests around where these are called, without changing blurb's functionality.

Then switch blurb to using a dash instead of a space and update tests. I've attempted to let it still be able to read old news entries from directories with spaces, in case some old PRs get merged that still have spaces.

(Also add some type hints and remove some unused code.)

---

We could decide whether we want to batch git mv existing news entries from space to dashes for files in each branch.

We'll also need changes to https://github.com/python/blurb_it, but I think they're quite small: python/blurb_it#332.

We also have releases due next week (Monday, 2023-04-03) for 3.10, 3.11 and 3.12, so let's wait for those to go out before merge. But I welcome reviews/testing already.

[CAM-Gerlach]

Not a blurb expert of course, but a couple comments on the CIs and test config...

Ideally, when testing this, it would be more robust to actually build and install the package (with e.g. python -m build then echo dist/*.whl | xargs -I % python -m pip install %) and then run python -I -m pytest to ensure you're testing what's actually installed rather than what happens to be in the working directory. That way, any packaging issues or similar don't fly under the radar (until after making a release).

[hugovk]

echo dist/*.whl | xargs -I % python -m pip install %

What do the %s do?

[CAM-Gerlach]

What do the %s do?

It's the conventional placeholder for the arg to be inserted with -I, in this case the package name. In that case AFAIK I could have just done echo dist/*.whl | xargs python -m pip install, but I typically use that form in my workflows as I often need to insert an optional dependency specifier (e.g. [test]) after the package name.

[CAM-Gerlach]

LGTM from my side, thanks @hugovk — but I'm very much not the expert on blurb, so hopefully someone who is will come along and review

[hugovk]

Testing an installed build drops coverage for blurb.py:

• https://app.codecov.io/gh/python/core-workflow
• https://github.com/python/core-workflow/actions/runs/4557364709/jobs/8038894379?pr=499#step:7:24

It's looking at the local blurb/blurb.py instead of the installed one. Do you know what's needed here?

[CAM-Gerlach]

Yeah, sorry, you need to set the basedir to the installed package dir, and possibly also execute the tests in a temporary directory rather than the project directory. Here's what worked in our QtPy test script to get the coverage and save the path to the installed project base directory, and here's what we did in our workflow to upload it—some of the complexity there won't be needed since you're not concatenating multiple sub-runs on the same job runner.

[hugovk]

Ah, in that case let's go for simple editable install, I don't see basedir options for pytest-cov/coverage for local dev or for codecov for CI, and editable is good enough for this.

[CAM-Gerlach]

I'm sure its possible, but yeah it does make configuring code coverage more complex particularly if you aren't using workflow tooling like Tox/Nox, so yeah not worth overcomplicating this PR with for now.

[larryhastings]

Hugo and I sprinted about this today at EuroPython. We changed some things.

• Back when I wrote blurb, I consciously said "yes, let's have directories with spaces in them", because it was the far-off futuristic year of 2016 or something, and I thought finally we could live with spaces in directory names. I am crestfallen that here we are in 2023 and we've decided we still aren't there. I'm not going to push back on this change--it doesn't really matter, and if it caused pain for people then sure let's change it. I just wanted to indulge in a moment of sadness. We can put a man on the moon, and robots crawl all over Mars, and cars can drive themselves, and eyeglasses are now multicore computers, and we still can't have spaces in directory names. *sob*
• I disagreed that replacing " " (space) with "-" (dash) was best, I preferred "_" (underscore). That's the natural, intuitive replacement for spaces if you don't want spaces. Hugo quietly acquiesced, and the patch now uses underscores in the section name directories (e.g. "Code_and_Builtins").
• I realized quickly that Hugo's implementation had a bug. If there were blurbs both with the old section directory name ("Core and Builtins") and with the new section directory name ("Core_and_Builtins"), when the release manager ran "blurb release" they'd be sorted separately and concatenated. This would almost certainly result in entries being listed out-of-order. It was, eventually, an easy fix.
• I stumbled over a not-so-great UX: instead of adding the GitHub issue # to the end of the ".. gh-issue:" line, I put it on the next line--and blurb complained about a missing section. This is because the blurb parser saw the line that didn't start with ".." and said oh! we're in the body section now. Then when we checked that it was valid we checked the section first. I changed error handling for parsing blurb files to check things in order and raise on the first error it sees, which is the UX folks expect. I also added rudimentary tests for this (input files in the "tests/fail" directory that trip over a couple of handled errors).

---

After we make this change in blurb, and change the directory names in all the CPython branches, the next step is to get all of CPython core to update their local copy of blurb. I bet that's not going to happen immediately. If someone (me? Victor Stinner?) still has an old copy of "blurb" installed somewhere, it'll happily write to the old directory name with spaces. I bet those are going to keep cropping up for a while.

So here's a blue-sky idea for you. As part of this PR, we add a check to blurb that confirms blurb is permitted to write to that directory in "next". Something like, checking to see if there's a "README.rst" in that directory name. And maybe we could write a magic cookie string into the README.rst as a comment, and confirm that it's there. And if it's not there, have blurb refuse to write to that directory, printing an error like "maybe either your Python tree or your copy of blurb is out of date?".

This approach would hopefully? mostly? future-proof blurb against this sort of thing happening in the future. If current blurb did this, we could rename the directories, and suddenly blurb would notice that "Core and Builtins/README.rst" didn't exist and it would complain.

---

This also makes me think we should do a nice cleanup pass on blurb, killing all the PRs and issues, before we actually make a release. I bet most people haven't upgraded their blurb in years; if we're going to ask them to go to the trouble, we should really dot all our i's and cross all our t's. Maybe we can take this opportunity to do some cleaning on blurb itself, like converting to pathlib.

How about we shoot for the next release of blurb happening on 3.12 final release day? (or maybe rc1?) Just seems like a day core devs will be paying a little more attention, maybe.

p.s. the nice thing is, we'll know exactly who is using the old blurb that is still writing to "Core and Builtins"

[hugovk]

One reason for hyphens over underscores: often underscores are treated like letters when doing things like option-arrow (Mac) or ctrl-arrow (Windows) to hop over words, whereas hyphens are often treated more like spaces. Anyway, not a big deal :)

---

Rather than putting a check in blurb to read a directory's README.rst to check if it's allowed to write there, I think a more general approach would be a CI check.

For example, we can add this to .pre-commit-config.yaml in each CPython branch once we clean it:

  -   repo: local
      hooks:
      -   id: news-dirs
          name: Check NEWS directories
          entry: Move NEWS to C_API or Core_and_Builtins directory
          language: fail
          files: '^Misc/NEWS.d/next/(C API)|(Core and Builtins)/.*\.rst$'

This will fail if it finds any matching files (docs: https://pre-commit.com/#fail).

Putting the check in the CI means:

• It will catch all tools including blurb: blurb-it, manual news file creation, etc.
• We don't need to wait for people to update their local copy of blurb before it triggers
• It's always in sync with the branch
• Will also catch old PRs, if we update their branches before merge

---

Releasing: I generally prefer an approach of release early, release often. Releasing to PyPI is easy (we can also automate it to make it even easier). Converting to pathlib is good, although more of an internal change so not essential. Having said that:

(or maybe rc1?)

RC1 is next week, so that's fine by me :) I would recommend releasing just after the RC1 rather than before, I wouldn't want to cause any RM hassles.

[hugovk]

Conflict resolved, ready for review.

[ezio-melotti]

To make things more explicit/readable, this could use a dict similar to the one used below for unsanitize_section.
If new sections that require sanitation are added in the future the function won't work, but it will serve as a reminder to update both dicts.

[hugovk]

Updated.

[vstinner]

I'm annoyed very often by this issue. Thanks for working on a fix!