Mark Builder.write() as final

[AA-Turner]

Resolves #12736, #12680.

We introduce a new method, Builder.write_documents() to oversee writing docnames, in-between Builder.write() and Builder.write_doc(). Builder.write() is then marked as final, and existing users of .write() are migrated to the new function.

An alternative design would be to introduce a new method in-between Builder.build() and Builder.write(), effectively renaming write_documents in this PR to write and write to a new name. That comes with risks, though, as if copying assets and preparing writing is not idempotent, it will not be backwards compatible.

The current design is backwards compatible as users overriding Builder.write() will observe no runtime failures, but will recieve new static typing failures. The migration route is to use Builder.write_documents().

A

[chrisjsewell]

Thanks for the review request @AA-Turner looks interesting 😄 hopefully will be able to have a look within the next few days

[AA-Turner]

Friendly ping @chrisjsewell -- interested in your thoughts on this!

A

[chrisjsewell]

Friendly ping @chrisjsewell -- interested in your thoughts on this!

Sorry for the delay, I got a bit side-tracked with other things 🫣
Hope to give this a proper look over the weekend or Monday 😅

[AA-Turner]

Friendly ping again @chrisjsewell if you have any time. Sorry I've let this languish for a fortnight, meant to comment earlier!

A

[AA-Turner]

October ping @chrisjsewell

A

[chrisjsewell]

October ping @chrisjsewell

Hey @AA-Turner sorry let me have a quick look over this in the next hour, before you merge 😄

[AA-Turner]

Thanks Chris -- I was going to write to you to ask if this should be left out of 8.1, so if you have time now to review that's tremendous.

A

[chrisjsewell]

Thanks Chris -- I was going to write to you to ask if this should be left out of 8.1, so if you have time now to review that's tremendous.

A

yep, just about finished a big release for sphinx-needs that diverted my attention 😅, which this certainly has a bearing on this useblocks/sphinx-needs#1326

[chrisjsewell]

Just some nitpicks, but yeh I think good in principle cheers

[chrisjsewell]

Oh one other thing: should we make Builder an https://docs.python.org/3/library/abc.html#abc.ABC class, with @abstractmethod rather than using raise NotImplementedError?